fix Rust _bind_mount_darwin_dmg()

Summary: We used to use disk images for redirections, but the code had long since gone stale. This diff fixes some of the logic so that the disk image code path will work again.

Reviewed By: fanzeyi

Differential Revision: D42467863

fbshipit-source-id: 12927aad617ffb3fb805dcbb91e1a671943fd1c0
This commit is contained in:
Michael Cuevas 2023-02-14 12:29:56 -08:00 committed by Facebook GitHub Bot
parent 1794f13642
commit 6000312f1a
4 changed files with 114 additions and 45 deletions

View File

@ -50,6 +50,29 @@ def have_apfs_helper() -> bool:
return False
def determine_bind_redirection_type(instance: EdenInstance) -> str:
"""Determine what bind redirection type should be used on macOS.
There are currently only 2 options: "apfs" or "dmg". We default
to the old behavior, "apfs"."""
config_value = instance.get_config_value(
"redirections.darwin-redirection-type", "apfs"
)
default_type = "apfs" if have_apfs_helper() else "dmg"
if config_value not in ["apfs", "dmg"]:
print(
f'darwin redirection type {config_value} must be either "apfs" or "dmg". Defaulting to {default_type}.'
)
config_value = default_type
if config_value == "apfs" and not have_apfs_helper():
print(
f"cannot use apfs redirections since apfs_helper '{APFS_HELPER}' is not available. Defaulting to {default_type} redirections."
)
config_value = default_type
return config_value
def is_bind_mount(path: Path) -> bool:
"""Detect the most common form of a bind mount in the repo;
its parent directory will have a different device number than
@ -188,7 +211,10 @@ class Redirection:
def expand_target_abspath(self, checkout: EdenCheckout) -> Optional[Path]:
if self.type == RedirectionType.BIND:
if have_apfs_helper():
if (
sys.platform == "darwin"
and determine_bind_redirection_type(checkout.instance) == "apfs"
):
# Ideally we'd return information about the backing, but
# it is a bit awkward to determine this in all contexts;
# prior to creating the volume we don't know anything
@ -226,10 +252,10 @@ class Redirection:
def _bind_mount_darwin(
self, instance: EdenInstance, checkout_path: Path, target: Path
) -> None:
if have_apfs_helper():
return self._bind_mount_darwin_apfs(instance, checkout_path, target)
else:
if determine_bind_redirection_type(instance) == "dmg":
return self._bind_mount_darwin_dmg(instance, checkout_path, target)
else:
return self._bind_mount_darwin_apfs(instance, checkout_path, target)
def _bind_mount_darwin_apfs(
self, instance: EdenInstance, checkout_path: Path, target: Path
@ -254,6 +280,8 @@ class Redirection:
total_kb = total / 1024
mount_path = checkout_path / self.repo_path
if not image_file_name.exists():
if not image_file_name.parent.exists():
image_file_name.parent.mkdir(exist_ok=True, parents=True)
run_cmd_quietly(
[
"hdiutil",
@ -270,6 +298,8 @@ class Redirection:
]
)
if not mount_path.parent.exists():
mount_path.parent.mkdir(exist_ok=True, parents=True)
run_cmd_quietly(
[
"hdiutil",
@ -283,8 +313,11 @@ class Redirection:
def _bind_unmount_darwin(self, checkout: EdenCheckout) -> None:
mount_path = checkout.path / self.repo_path
# This will unmount/detach both disk images and apfs volumes
run_cmd_quietly(["diskutil", "unmount", "force", mount_path])
# This will unmount/detach both disk images and apfs volumes.
# `diskutil eject` fully removes the image from the list of disk
# partitions while `diskutil unmount` leaves leftover state for some
# reason. We will use `eject` since they're essentially the same.
run_cmd_quietly(["diskutil", "eject", mount_path])
def _bind_mount_linux(
self, instance: EdenInstance, checkout_path: Path, target: Path

View File

@ -42,6 +42,7 @@ tempfile = "3.3"
[target.'cfg(target_os = "macos")'.dependencies]
nix = "0.25"
psutil = "3.2.2"
[target.'cfg(target_os = "windows")'.dependencies]
mkscratch = { version = "0.1.0", path = "../../../scm/exec/scratch" }

View File

@ -35,9 +35,9 @@ use edenfs_utils::stop_buckd_for_repo;
use fbinit::expect_init;
#[cfg(target_os = "windows")]
use mkscratch::zzencode;
#[cfg(target_os = "macos")]
use nix::sys::stat::stat;
use pathdiff::diff_paths;
#[cfg(target_os = "macos")]
use psutil::disk::disk_usage;
use serde::Deserialize;
use serde::Deserializer;
use serde::Serialize;
@ -352,8 +352,9 @@ impl Redirection {
pub fn expand_target_abspath(&self, checkout: &EdenFsCheckout) -> Result<Option<PathBuf>> {
match self.redir_type {
#[cfg(target_os = "macos")]
RedirectionType::Bind => {
if Redirection::have_apfs_helper()? {
if Self::determine_bind_redirection_type() == DarwinBindRedirectionType::APFS {
// Ideally we'd return information about the backing, but
// it is a bit awkward to determine this in all contexts;
// prior to creating the volume we don't know anything
@ -380,6 +381,11 @@ impl Redirection {
)?))
}
}
#[cfg(not(target_os = "macos"))]
RedirectionType::Bind => Ok(Some(Redirection::make_scratch_dir(
checkout,
&self.repo_path,
)?)),
RedirectionType::Symlink => Ok(Some(Redirection::make_scratch_dir(
checkout,
&self.repo_path,
@ -477,31 +483,44 @@ impl Redirection {
// Since we don't have bind mounts, we set up a disk image file
// and mount that instead.
let image_file_path = self._dmg_file_name(target);
let target_stat = stat(target)
let target_stat = disk_usage(target)
.from_err()
.with_context(|| format!("Failed to stat target {}", target.display()))?;
// Specify the size in kb because the disk utilities have weird
// defaults if the units are unspecified, and `b` doesn't mean
// bytes!
let total_kb = target_stat.st_size / 1024;
let total_kib = target_stat.total() / 1024;
let mount_path = checkout_path.join(&self.repo_path());
if !image_file_path.exists() {
// We need to convert paths -> strings for the hdiutil commands
let image_file_name = image_file_path.to_string_lossy();
let mount_name = mount_path.to_string_lossy();
// We need to convert paths -> strings for the hdiutil commands
let image_file_name = image_file_path.to_string_lossy();
let mount_name = mount_path.to_string_lossy();
if !image_file_path.exists() {
let image_file_dir = image_file_path.parent().with_context(|| {
format!(
"image file {} must exist in some parent directory",
&image_file_path.display()
)
})?;
if !image_file_dir.exists() {
std::fs::create_dir_all(&image_file_dir)
.from_err()
.with_context(|| {
format!("Failed to create directory {}", &image_file_dir.display())
})?;
}
let args = &[
"create",
"--size",
&format!("{}k", total_kb),
"--type",
"-size",
&format!("{}k", total_kib),
"-type",
"SPARSE",
"--fs",
"-fs",
"HFS+",
"--volname",
&format!("EdenFS redirection for {}", &mount_name),
"-volname",
&format!("'EdenFS redirection for {}'", &mount_name),
&image_file_name,
];
let create_output = Command::new("hdiutil")
@ -520,30 +539,38 @@ impl Redirection {
String::from_utf8_lossy(&create_output.stdout)
)));
}
let args = &[
"attach",
&image_file_name,
"--nobrowse",
"--mountpoint",
&mount_name,
];
let attach_output = Command::new("hdiutil")
.args(args)
.output()
}
let args = &[
"attach",
&image_file_name,
"-nobrowse",
"-mountpoint",
&mount_name,
];
let mount_path = mount_path.parent().with_context(|| {
format!(
"mount path {} must exist in some parent directory",
&mount_path.display()
)
})?;
if !mount_path.exists() {
std::fs::create_dir_all(&mount_path)
.from_err()
.with_context(|| {
format!("Failed to execute command `hdiutil {}`", args.join(" "))
})?;
if !attach_output.status.success() {
return Err(EdenFsError::Other(anyhow!(
"failed to attach dmg volume {} for mount {}. stderr: {}\n stdout: {}",
&image_file_name,
&mount_name,
String::from_utf8_lossy(&attach_output.stderr),
String::from_utf8_lossy(&attach_output.stdout)
)));
}
.with_context(|| format!("Failed to create directory {}", &mount_path.display()))?;
}
let attach_output = Command::new("hdiutil")
.args(args)
.output()
.from_err()
.with_context(|| format!("Failed to execute command `hdiutil {}`", args.join(" ")))?;
if !attach_output.status.success() {
return Err(EdenFsError::Other(anyhow!(
"failed to attach dmg volume {} for mount {}. stderr: {}\n stdout: {}",
&image_file_name,
&mount_name,
String::from_utf8_lossy(&attach_output.stderr),
String::from_utf8_lossy(&attach_output.stdout)
)));
}
Ok(())
}
@ -606,7 +633,10 @@ impl Redirection {
#[cfg(target_os = "macos")]
fn _bind_unmount_darwin(&self, checkout: &EdenFsCheckout) -> Result<()> {
let mount_path = checkout.path().join(&self.repo_path);
let args = &["unmount", "force", &mount_path.to_string_lossy()];
// `diskutil eject` fully removes the image from the list of disk partitions while
// `diskutil unmount` leaves leftover state for some reason. We will use `eject` since
// they're essentially the same.
let args = &["eject", &mount_path.to_string_lossy()];
let output = Command::new("diskutil")
.args(args)
.output()

View File

@ -256,6 +256,11 @@ class EdenTestCase(EdenTestCaseBase):
configs = {"experimental": ["enable-nfs-server = true"]}
if self.use_nfs():
configs["clone"] = ['default-mount-protocol = "NFS"']
# The number of concurrent APFS volumes we can create on macOS
# Sandcastle hosts is extremely limited. Therefore, let's use disk
# image redirections instead of APFS volume redirections.
if sys.platform == "darwin":
configs["redirections"] = ['darwin-redirection-type = "dmg"']
return configs
def create_hg_repo(