Skip to content

Commit fc877dd

Browse files
committed
[hyperactor] mesh: make it work with cargo on a mac
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. 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/)! ghstack-source-id: 314019054 Pull Request resolved: #1425
1 parent e5ca0a9 commit fc877dd

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
@@ -1321,9 +1321,9 @@ mod tests {
13211321
use crate::alloc::process::ProcessAllocator;
13221322

13231323
fn process_allocator() -> ProcessAllocator {
1324-
ProcessAllocator::new(Command::new(
1325-
buck_resources::get("monarch/hyperactor_mesh/bootstrap").unwrap(),
1326-
))
1324+
ProcessAllocator::new(Command::new(crate::testresource::get(
1325+
"monarch/hyperactor_mesh/bootstrap",
1326+
)))
13271327
}
13281328

13291329
#[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
@@ -710,8 +710,9 @@ pub(crate) mod testing {
710710
Duration::from_secs(1),
711711
);
712712

713-
let command =
714-
Command::new(buck_resources::get("monarch/hyperactor_mesh/bootstrap").unwrap());
713+
let command = Command::new(crate::testresource::get(
714+
"monarch/hyperactor_mesh/bootstrap",
715+
));
715716
let mut allocator = ProcessAllocator::new(command);
716717
let mut alloc = allocator
717718
.allocate(AllocSpec {

hyperactor_mesh/src/alloc/process.rs

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

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

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

612612
let mut alloc = allocator

hyperactor_mesh/src/alloc/remoteprocess.rs

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2048,13 +2048,15 @@ mod test_alloc {
20482048
let task1_allocator = RemoteProcessAllocator::new();
20492049
let task1_addr = ChannelAddr::any(ChannelTransport::Unix);
20502050
let task1_addr_string = task1_addr.to_string();
2051-
let task1_cmd =
2052-
Command::new(buck_resources::get("monarch/hyperactor_mesh/bootstrap").unwrap());
2051+
let task1_cmd = Command::new(crate::testresource::get(
2052+
"monarch/hyperactor_mesh/bootstrap",
2053+
));
20532054
let task2_allocator = RemoteProcessAllocator::new();
20542055
let task2_addr = ChannelAddr::any(ChannelTransport::Unix);
20552056
let task2_addr_string = task2_addr.to_string();
2056-
let task2_cmd =
2057-
Command::new(buck_resources::get("monarch/hyperactor_mesh/bootstrap").unwrap());
2057+
let task2_cmd = Command::new(crate::testresource::get(
2058+
"monarch/hyperactor_mesh/bootstrap",
2059+
));
20582060
let task1_allocator_copy = task1_allocator.clone();
20592061
let task1_allocator_handle = tokio::spawn(async move {
20602062
tracing::info!("spawning task1");
@@ -2175,13 +2177,15 @@ mod test_alloc {
21752177
let task1_allocator = RemoteProcessAllocator::new();
21762178
let task1_addr = ChannelAddr::any(ChannelTransport::Unix);
21772179
let task1_addr_string = task1_addr.to_string();
2178-
let task1_cmd =
2179-
Command::new(buck_resources::get("monarch/hyperactor_mesh/bootstrap").unwrap());
2180+
let task1_cmd = Command::new(crate::testresource::get(
2181+
"monarch/hyperactor_mesh/bootstrap",
2182+
));
21802183
let task2_allocator = RemoteProcessAllocator::new();
21812184
let task2_addr = ChannelAddr::any(ChannelTransport::Unix);
21822185
let task2_addr_string = task2_addr.to_string();
2183-
let task2_cmd =
2184-
Command::new(buck_resources::get("monarch/hyperactor_mesh/bootstrap").unwrap());
2186+
let task2_cmd = Command::new(crate::testresource::get(
2187+
"monarch/hyperactor_mesh/bootstrap",
2188+
));
21852189
let task1_allocator_copy = task1_allocator.clone();
21862190
let task1_allocator_handle = tokio::spawn(async move {
21872191
tracing::info!("spawning task1");
@@ -2303,8 +2307,9 @@ mod test_alloc {
23032307
let task1_allocator = RemoteProcessAllocator::new();
23042308
let task1_addr = ChannelAddr::any(ChannelTransport::Unix);
23052309
let task1_addr_string = task1_addr.to_string();
2306-
let task1_cmd =
2307-
Command::new(buck_resources::get("monarch/hyperactor_mesh/bootstrap").unwrap());
2310+
let task1_cmd = Command::new(crate::testresource::get(
2311+
"monarch/hyperactor_mesh/bootstrap",
2312+
));
23082313
let task2_allocator = RemoteProcessAllocator::new();
23092314
let task2_addr = ChannelAddr::any(ChannelTransport::Unix);
23102315
let task2_addr_string = task2_addr.to_string();
@@ -2427,10 +2432,9 @@ mod test_alloc {
24272432
let remote_process_allocators = addresses
24282433
.iter()
24292434
.map(|addr| {
2430-
Command::new(
2431-
buck_resources::get("monarch/hyperactor_mesh/remote_process_allocator")
2432-
.unwrap(),
2433-
)
2435+
Command::new(crate::testresource::get(
2436+
"monarch/hyperactor_mesh/remote_process_allocator",
2437+
))
24342438
.env("RUST_LOG", "info")
24352439
.arg(format!("--addr={addr}"))
24362440
.stdout(std::process::Stdio::piped())
@@ -2442,9 +2446,9 @@ mod test_alloc {
24422446
let done_allocating_addr = ChannelAddr::any(ChannelTransport::Unix);
24432447
let (done_allocating_addr, mut done_allocating_rx) =
24442448
channel::serve::<()>(done_allocating_addr).unwrap();
2445-
let mut remote_process_alloc = Command::new(
2446-
buck_resources::get("monarch/hyperactor_mesh/remote_process_alloc").unwrap(),
2447-
)
2449+
let mut remote_process_alloc = Command::new(crate::testresource::get(
2450+
"monarch/hyperactor_mesh/remote_process_alloc",
2451+
))
24482452
.arg(format!("--done-allocating-addr={}", done_allocating_addr))
24492453
.arg(format!("--addresses={}", addresses.join(",")))
24502454
.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
@@ -396,11 +396,14 @@ impl Bootstrap {
396396

397397
/// Install "kill me if parent dies" and close the race window.
398398
pub fn install_pdeathsig_kill() -> io::Result<()> {
399-
// SAFETY: Calling into libc; does not dereference memory, just
400-
// asks the kernel to deliver SIGKILL on parent death.
401-
let rc = unsafe { libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGKILL as c_int) };
402-
if rc != 0 {
403-
return Err(io::Error::last_os_error());
399+
#[cfg(target_os = "linux")]
400+
{
401+
// SAFETY: Calling into libc; does not dereference memory, just
402+
// asks the kernel to deliver SIGKILL on parent death.
403+
let rc = unsafe { libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGKILL as c_int) };
404+
if rc != 0 {
405+
return Err(io::Error::last_os_error());
406+
}
404407
}
405408
// Race-close: if the parent died between our exec and prctl(),
406409
// we won't get a signal, so detect that and exit now.
@@ -1343,7 +1346,7 @@ impl BootstrapCommand {
13431346
#[cfg(test)]
13441347
pub(crate) fn test() -> Self {
13451348
Self {
1346-
program: buck_resources::get("monarch/hyperactor_mesh/bootstrap").unwrap(),
1349+
program: crate::testresource::get("monarch/hyperactor_mesh/bootstrap"),
13471350
arg0: None,
13481351
args: vec![],
13491352
env: HashMap::new(),
@@ -2548,7 +2551,7 @@ mod tests {
25482551
let manager = BootstrapProcManager::new(command);
25492552

25502553
// Spawn a fast-exiting child.
2551-
let mut cmd = Command::new("/bin/true");
2554+
let mut cmd = Command::new("true");
25522555
cmd.stdout(Stdio::null()).stderr(Stdio::null());
25532556
let child = cmd.spawn().expect("spawn true");
25542557

@@ -3225,9 +3228,9 @@ mod tests {
32253228
let (instance, _handle) = proc.instance("client").unwrap();
32263229

32273230
// Configure a ProcessAllocator with the bootstrap binary.
3228-
let mut allocator = ProcessAllocator::new(Command::new(
3229-
buck_resources::get("monarch/hyperactor_mesh/bootstrap").unwrap(),
3230-
));
3231+
let mut allocator = ProcessAllocator::new(Command::new(crate::testresource::get(
3232+
"monarch/hyperactor_mesh/bootstrap",
3233+
)));
32313234
// Request a new allocation of procs from the ProcessAllocator.
32323235
let alloc = allocator
32333236
.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
@@ -63,9 +63,9 @@ pub async fn proc_meshes(cx: &impl context::Actor, extent: Extent) -> Vec<ProcMe
6363
});
6464

6565
meshes.push({
66-
let mut allocator = ProcessAllocator::new(Command::new(
67-
buck_resources::get("monarch/hyperactor_mesh/bootstrap").unwrap(),
68-
));
66+
let mut allocator = ProcessAllocator::new(Command::new(crate::testresource::get(
67+
"monarch/hyperactor_mesh/bootstrap",
68+
)));
6969
let alloc = allocator
7070
.allocate(AllocSpec {
7171
extent,
@@ -94,9 +94,9 @@ pub async fn allocs(extent: Extent) -> Vec<Box<dyn Alloc + Send + Sync>> {
9494
vec![
9595
// Box::new(LocalAllocator.allocate(spec.clone()).await.unwrap()),
9696
Box::new(
97-
ProcessAllocator::new(Command::new(
98-
buck_resources::get("monarch/hyperactor_mesh/bootstrap").unwrap(),
99-
))
97+
ProcessAllocator::new(Command::new(crate::testresource::get(
98+
"monarch/hyperactor_mesh/bootstrap",
99+
)))
100100
.allocate(spec.clone())
101101
.await
102102
.unwrap(),
@@ -130,9 +130,9 @@ pub async fn local_proc_mesh(extent: Extent) -> (ProcMesh, Instance<()>, DialMai
130130

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

0 commit comments

Comments
 (0)