Skip to content

Commit 4869419

Browse files
committed
[hyperactor] mesh: make it work with cargo on a mac
Pull Request resolved: #1425 1) PR_DEATHSIG is a Linux feature; hide it behind a feature flag 2) heuristically find cargo-built resources in place of buck Note that (2) is not a true solution: Cargo does not take these binaries as a dependence on the tests, and thus will not automatically rebuild them. Cargo apparently will do this for integration tests, however; we should consider converting the relevant tests to integration tests. ghstack-source-id: 314156439 Differential Revision: [D83882828](https://our.internmc.facebook.com/intern/diff/D83882828/) **NOTE FOR REVIEWERS**: This PR has internal Meta-specific changes or comments, please review them on [Phabricator](https://our.internmc.facebook.com/intern/diff/D83882828/)!
1 parent 6e87ff0 commit 4869419

File tree

9 files changed

+102
-44
lines changed

9 files changed

+102
-44
lines changed

hyperactor_mesh/src/actor_mesh.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1345,9 +1345,9 @@ mod tests {
13451345
use crate::alloc::process::ProcessAllocator;
13461346

13471347
fn process_allocator() -> ProcessAllocator {
1348-
ProcessAllocator::new(Command::new(
1349-
buck_resources::get("monarch/hyperactor_mesh/bootstrap").unwrap(),
1350-
))
1348+
ProcessAllocator::new(Command::new(crate::testresource::get(
1349+
"monarch/hyperactor_mesh/bootstrap",
1350+
)))
13511351
}
13521352

13531353
#[cfg(fbcode_build)] // we use an external binary, produced by buck

hyperactor_mesh/src/alloc.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -713,8 +713,9 @@ pub(crate) mod testing {
713713
Duration::from_secs(1),
714714
);
715715

716-
let command =
717-
Command::new(buck_resources::get("monarch/hyperactor_mesh/bootstrap").unwrap());
716+
let command = Command::new(crate::testresource::get(
717+
"monarch/hyperactor_mesh/bootstrap",
718+
));
718719
let mut allocator = ProcessAllocator::new(command);
719720
let mut alloc = allocator
720721
.allocate(AllocSpec {

hyperactor_mesh/src/alloc/process.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -603,12 +603,12 @@ mod tests {
603603

604604
#[cfg(fbcode_build)] // we use an external binary, produced by buck
605605
crate::alloc_test_suite!(ProcessAllocator::new(Command::new(
606-
buck_resources::get("monarch/hyperactor_mesh/bootstrap").unwrap()
606+
crate::testresource::get("monarch/hyperactor_mesh/bootstrap")
607607
)));
608608

609609
#[tokio::test]
610610
async fn test_sigterm_on_group_fail() {
611-
let bootstrap_binary = buck_resources::get("monarch/hyperactor_mesh/bootstrap").unwrap();
611+
let bootstrap_binary = crate::testresource::get("monarch/hyperactor_mesh/bootstrap");
612612
let mut allocator = ProcessAllocator::new(Command::new(bootstrap_binary));
613613

614614
let mut alloc = allocator

hyperactor_mesh/src/alloc/remoteprocess.rs

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2042,13 +2042,15 @@ mod test_alloc {
20422042
let task1_allocator = RemoteProcessAllocator::new();
20432043
let task1_addr = ChannelAddr::any(ChannelTransport::Unix);
20442044
let task1_addr_string = task1_addr.to_string();
2045-
let task1_cmd =
2046-
Command::new(buck_resources::get("monarch/hyperactor_mesh/bootstrap").unwrap());
2045+
let task1_cmd = Command::new(crate::testresource::get(
2046+
"monarch/hyperactor_mesh/bootstrap",
2047+
));
20472048
let task2_allocator = RemoteProcessAllocator::new();
20482049
let task2_addr = ChannelAddr::any(ChannelTransport::Unix);
20492050
let task2_addr_string = task2_addr.to_string();
2050-
let task2_cmd =
2051-
Command::new(buck_resources::get("monarch/hyperactor_mesh/bootstrap").unwrap());
2051+
let task2_cmd = Command::new(crate::testresource::get(
2052+
"monarch/hyperactor_mesh/bootstrap",
2053+
));
20522054
let task1_allocator_copy = task1_allocator.clone();
20532055
let task1_allocator_handle = tokio::spawn(async move {
20542056
tracing::info!("spawning task1");
@@ -2169,13 +2171,15 @@ mod test_alloc {
21692171
let task1_allocator = RemoteProcessAllocator::new();
21702172
let task1_addr = ChannelAddr::any(ChannelTransport::Unix);
21712173
let task1_addr_string = task1_addr.to_string();
2172-
let task1_cmd =
2173-
Command::new(buck_resources::get("monarch/hyperactor_mesh/bootstrap").unwrap());
2174+
let task1_cmd = Command::new(crate::testresource::get(
2175+
"monarch/hyperactor_mesh/bootstrap",
2176+
));
21742177
let task2_allocator = RemoteProcessAllocator::new();
21752178
let task2_addr = ChannelAddr::any(ChannelTransport::Unix);
21762179
let task2_addr_string = task2_addr.to_string();
2177-
let task2_cmd =
2178-
Command::new(buck_resources::get("monarch/hyperactor_mesh/bootstrap").unwrap());
2180+
let task2_cmd = Command::new(crate::testresource::get(
2181+
"monarch/hyperactor_mesh/bootstrap",
2182+
));
21792183
let task1_allocator_copy = task1_allocator.clone();
21802184
let task1_allocator_handle = tokio::spawn(async move {
21812185
tracing::info!("spawning task1");
@@ -2297,8 +2301,9 @@ mod test_alloc {
22972301
let task1_allocator = RemoteProcessAllocator::new();
22982302
let task1_addr = ChannelAddr::any(ChannelTransport::Unix);
22992303
let task1_addr_string = task1_addr.to_string();
2300-
let task1_cmd =
2301-
Command::new(buck_resources::get("monarch/hyperactor_mesh/bootstrap").unwrap());
2304+
let task1_cmd = Command::new(crate::testresource::get(
2305+
"monarch/hyperactor_mesh/bootstrap",
2306+
));
23022307
let task2_allocator = RemoteProcessAllocator::new();
23032308
let task2_addr = ChannelAddr::any(ChannelTransport::Unix);
23042309
let task2_addr_string = task2_addr.to_string();
@@ -2421,10 +2426,9 @@ mod test_alloc {
24212426
let remote_process_allocators = addresses
24222427
.iter()
24232428
.map(|addr| {
2424-
Command::new(
2425-
buck_resources::get("monarch/hyperactor_mesh/remote_process_allocator")
2426-
.unwrap(),
2427-
)
2429+
Command::new(crate::testresource::get(
2430+
"monarch/hyperactor_mesh/remote_process_allocator",
2431+
))
24282432
.env("RUST_LOG", "info")
24292433
.arg(format!("--addr={addr}"))
24302434
.stdout(std::process::Stdio::piped())
@@ -2436,9 +2440,9 @@ mod test_alloc {
24362440
let done_allocating_addr = ChannelAddr::any(ChannelTransport::Unix);
24372441
let (done_allocating_addr, mut done_allocating_rx) =
24382442
channel::serve::<()>(done_allocating_addr).unwrap();
2439-
let mut remote_process_alloc = Command::new(
2440-
buck_resources::get("monarch/hyperactor_mesh/remote_process_alloc").unwrap(),
2441-
)
2443+
let mut remote_process_alloc = Command::new(crate::testresource::get(
2444+
"monarch/hyperactor_mesh/remote_process_alloc",
2445+
))
24422446
.arg(format!("--done-allocating-addr={}", done_allocating_addr))
24432447
.arg(format!("--addresses={}", addresses.join(",")))
24442448
.arg(format!("--num-proc-meshes={}", num_proc_meshes))

hyperactor_mesh/src/bootstrap.rs

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -401,11 +401,14 @@ impl Bootstrap {
401401

402402
/// Install "kill me if parent dies" and close the race window.
403403
pub fn install_pdeathsig_kill() -> io::Result<()> {
404-
// SAFETY: Calling into libc; does not dereference memory, just
405-
// asks the kernel to deliver SIGKILL on parent death.
406-
let rc = unsafe { libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGKILL as c_int) };
407-
if rc != 0 {
408-
return Err(io::Error::last_os_error());
404+
#[cfg(target_os = "linux")]
405+
{
406+
// SAFETY: Calling into libc; does not dereference memory, just
407+
// asks the kernel to deliver SIGKILL on parent death.
408+
let rc = unsafe { libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGKILL as c_int) };
409+
if rc != 0 {
410+
return Err(io::Error::last_os_error());
411+
}
409412
}
410413
// Race-close: if the parent died between our exec and prctl(),
411414
// we won't get a signal, so detect that and exit now.
@@ -1348,7 +1351,7 @@ impl BootstrapCommand {
13481351
#[cfg(test)]
13491352
pub(crate) fn test() -> Self {
13501353
Self {
1351-
program: buck_resources::get("monarch/hyperactor_mesh/bootstrap").unwrap(),
1354+
program: crate::testresource::get("monarch/hyperactor_mesh/bootstrap"),
13521355
arg0: None,
13531356
args: vec![],
13541357
env: HashMap::new(),
@@ -2553,7 +2556,7 @@ mod tests {
25532556
let manager = BootstrapProcManager::new(command);
25542557

25552558
// Spawn a fast-exiting child.
2556-
let mut cmd = Command::new("/bin/true");
2559+
let mut cmd = Command::new("true");
25572560
cmd.stdout(Stdio::null()).stderr(Stdio::null());
25582561
let child = cmd.spawn().expect("spawn true");
25592562

@@ -3230,9 +3233,9 @@ mod tests {
32303233
let (instance, _handle) = proc.instance("client").unwrap();
32313234

32323235
// Configure a ProcessAllocator with the bootstrap binary.
3233-
let mut allocator = ProcessAllocator::new(Command::new(
3234-
buck_resources::get("monarch/hyperactor_mesh/bootstrap").unwrap(),
3235-
));
3236+
let mut allocator = ProcessAllocator::new(Command::new(crate::testresource::get(
3237+
"monarch/hyperactor_mesh/bootstrap",
3238+
)));
32363239
// Request a new allocation of procs from the ProcessAllocator.
32373240
let alloc = allocator
32383241
.allocate(AllocSpec {

hyperactor_mesh/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ mod router;
3232
pub mod shared_cell;
3333
pub mod shortuuid;
3434
pub mod test_utils;
35+
mod testresource;
3536
pub mod v1;
3637

3738
pub use actor_mesh::RootActorMesh;
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
* All rights reserved.
4+
*
5+
* This source code is licensed under the BSD-style license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
#![cfg(test)]
10+
11+
use std::path::PathBuf;
12+
13+
/// Fetch the named (BUCK) named resource, heuristically falling back on
14+
/// the cargo-built path when possible. Beware! This is not actually a
15+
/// true cargo dependency, so the binaries have to be built independently.
16+
///
17+
/// We should convert these tests to integration tests, so that cargo can
18+
/// also manage the binaries.
19+
pub fn get<S>(name: S) -> PathBuf
20+
where
21+
S: AsRef<str>,
22+
{
23+
let name = name.as_ref().to_owned();
24+
// TODO: actually check if we're running in Buck context or not.
25+
if let Ok(path) = buck_resources::get(name.clone()) {
26+
return path;
27+
}
28+
29+
assert!(
30+
name.starts_with("monarch/hyperactor_mesh/"),
31+
"invalid resource {}: must start with \"monarch/hyperactor_mesh/\"",
32+
name
33+
);
34+
35+
let path: PathBuf = name
36+
.replace(
37+
"monarch/hyperactor_mesh/",
38+
"../target/debug/hyperactor_mesh_test_",
39+
)
40+
.into();
41+
42+
assert!(
43+
path.exists(),
44+
"no cargo-built resource at {}",
45+
path.display()
46+
);
47+
48+
path
49+
}

hyperactor_mesh/src/v1/host_mesh.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -831,7 +831,7 @@ mod tests {
831831
let config = hyperactor::config::global::lock();
832832
let _guard = config.override_key(crate::bootstrap::MESH_BOOTSTRAP_ENABLE_PDEATHSIG, false);
833833

834-
let program = buck_resources::get("monarch/hyperactor_mesh/bootstrap").unwrap();
834+
let program = crate::testresource::get("monarch/hyperactor_mesh/bootstrap");
835835

836836
let hosts = vec![free_localhost_addr(), free_localhost_addr()];
837837

hyperactor_mesh/src/v1/testing.rs

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,9 @@ pub async fn proc_meshes(cx: &impl context::Actor, extent: Extent) -> Vec<ProcMe
6565
});
6666

6767
meshes.push({
68-
let mut allocator = ProcessAllocator::new(Command::new(
69-
buck_resources::get("monarch/hyperactor_mesh/bootstrap").unwrap(),
70-
));
68+
let mut allocator = ProcessAllocator::new(Command::new(crate::testresource::get(
69+
"monarch/hyperactor_mesh/bootstrap",
70+
)));
7171
let alloc = allocator
7272
.allocate(AllocSpec {
7373
extent,
@@ -98,9 +98,9 @@ pub async fn allocs(extent: Extent) -> Vec<Box<dyn Alloc + Send + Sync>> {
9898
vec![
9999
// Box::new(LocalAllocator.allocate(spec.clone()).await.unwrap()),
100100
Box::new(
101-
ProcessAllocator::new(Command::new(
102-
buck_resources::get("monarch/hyperactor_mesh/bootstrap").unwrap(),
103-
))
101+
ProcessAllocator::new(Command::new(crate::testresource::get(
102+
"monarch/hyperactor_mesh/bootstrap",
103+
)))
104104
.allocate(spec.clone())
105105
.await
106106
.unwrap(),
@@ -135,9 +135,9 @@ pub async fn local_proc_mesh(extent: Extent) -> (ProcMesh, Instance<()>, DialMai
135135

136136
/// Create a host mesh using multiple processes running on the test machine.
137137
pub async fn host_mesh(extent: Extent) -> HostMesh {
138-
let mut allocator = ProcessAllocator::new(Command::new(
139-
buck_resources::get("monarch/hyperactor_mesh/bootstrap").unwrap(),
140-
));
138+
let mut allocator = ProcessAllocator::new(Command::new(crate::testresource::get(
139+
"monarch/hyperactor_mesh/bootstrap",
140+
)));
141141
let alloc = allocator
142142
.allocate(AllocSpec {
143143
extent,

0 commit comments

Comments
 (0)