Skip to content

Commit

Permalink
Merge #1207
Browse files Browse the repository at this point in the history
1207: Copy remote directories correctly r=Emilgardis a=Emilgardis

resolves #1206


Co-authored-by: Emil Gardström <emil.gardstrom@gmail.com>
  • Loading branch information
bors[bot] and Emilgardis authored Feb 12, 2023
2 parents fa1ab48 + af297cd commit 99b8069
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 10 deletions.
5 changes: 5 additions & 0 deletions .changes/1207.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"type": "fixed",
"description": "properly copy directories when using `CROSS_REMOTE`",
"issues": [1206]
}
44 changes: 34 additions & 10 deletions src/docker/remote.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,14 @@ impl<'a, 'b, 'c> ContainerDataVolume<'a, 'b, 'c> {
.run_and_get_status(msg_info, false)
}

// copy files for a docker volume, for remote host support
// NOTE: `reldst` has the same caveats as `reldir` in `create_dir`.
/// Copy files for a docker volume
///
/// `reldst` has the same caveats as `reldir` in [`Self::create_dir`].
///
/// ## Note
///
/// if copying from a src directory to dst directory with docker, to
/// copy the contents from `src` into `dst`, `src` must end with `/.`
#[track_caller]
fn copy_files(
&self,
Expand All @@ -67,14 +73,33 @@ impl<'a, 'b, 'c> ContainerDataVolume<'a, 'b, 'c> {
mount_prefix: &str,
msg_info: &mut MessageInfo,
) -> Result<ExitStatus> {
if let Some((_, rel)) = reldst.rsplit_once('/') {
if msg_info.cross_debug
&& src.is_dir()
&& !src.to_string_lossy().ends_with("/.")
&& rel == src.file_name().unwrap()
{
msg_info.warn(format_args!(
"source is pointing to a directory instead of its contents: {} -> {}\nThis might be a bug. {}",
src.as_posix_relative()?,
reldst,
std::panic::Location::caller()
))?;
}
}
subcommand_or_exit(self.engine, "cp")?
.arg("-a")
.arg(src.to_utf8()?)
.arg(format!("{}:{mount_prefix}/{reldst}", self.container))
.run_and_get_status(msg_info, false)
}

// copy files for a docker volume, for remote host support
/// copy files for a docker volume, does not include cache directories
///
/// ## Note
///
/// if copying from a src directory to dst directory with docker, to
/// copy the contents from `src` into `dst`, `src` must end with `/.`
#[track_caller]
fn copy_files_nocache(
&self,
Expand Down Expand Up @@ -114,8 +139,7 @@ impl<'a, 'b, 'c> ContainerDataVolume<'a, 'b, 'c> {
file::create_dir_all(dst_path.parent().expect("must have parent"))?;
fs::copy(src_path, &dst_path)?;
}
// if copying from a src directory to dst directory with docker, to
// copy the contents from `src` into `dst`, `src` must end with `/.`

self.copy_files(&temppath.join("."), reldst, mount_prefix, msg_info)
}

Expand Down Expand Up @@ -196,7 +220,7 @@ impl<'a, 'b, 'c> ContainerDataVolume<'a, 'b, 'c> {
mount_prefix,
msg_info,
)?;
self.copy_files(dirs.xargo(), &reldst, mount_prefix, msg_info)?;
self.copy_files(&dirs.xargo().join("."), &reldst, mount_prefix, msg_info)?;
}

Ok(())
Expand All @@ -215,11 +239,11 @@ impl<'a, 'b, 'c> ContainerDataVolume<'a, 'b, 'c> {
.map(|s| bool_from_envvar(&s))
.unwrap_or(copy_registry);

self.create_dir(&reldst, mount_prefix, msg_info)?;
if copy_registry {
self.copy_files(dirs.cargo(), &reldst, mount_prefix, msg_info)?;
self.copy_files(&dirs.cargo().join("."), &reldst, mount_prefix, msg_info)?;
} else {
// can copy a limit subset of files: the rest is present.
self.create_dir(&reldst, mount_prefix, msg_info)?;
for entry in fs::read_dir(dirs.cargo())
.wrap_err_with(|| format!("when reading directory {:?}", dirs.cargo()))?
{
Expand Down Expand Up @@ -380,9 +404,9 @@ impl<'a, 'b, 'c> ContainerDataVolume<'a, 'b, 'c> {
) -> Result<()> {
let copy_all = |info: &mut MessageInfo| {
if copy_cache {
self.copy_files(src, reldst, mount_prefix, info)
self.copy_files(&src.join("."), reldst, mount_prefix, info)
} else {
self.copy_files_nocache(src, reldst, mount_prefix, true, info)
self.copy_files_nocache(&src.join("."), reldst, mount_prefix, true, info)
}
};
match volume {
Expand Down

0 comments on commit 99b8069

Please sign in to comment.