-
Notifications
You must be signed in to change notification settings - Fork 379
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Copy remote directories correctly #1207
Conversation
src/docker/remote.rs
Outdated
if let Some((_, rel)) = reldst.rsplit_once('/') { | ||
if src.is_dir() && src.file_name().unwrap().to_utf8()? == rel { | ||
eyre::bail!( | ||
"reldst is pointing to the wrong path: {} -> {}\n{}", | ||
src.as_posix_relative()?, | ||
reldst, | ||
std::panic::Location::caller() | ||
); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be behind CROSS_DEBUG
src/docker/remote.rs
Outdated
dirs.cargo(), | ||
reldst | ||
.rsplit_once('/') | ||
.ok_or_else(|| eyre::eyre!("cargo can't be a root directory"))? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.ok_or_else(|| eyre::eyre!("cargo can't be a root directory"))? | |
.ok_or_else(|| eyre::eyre!("can not be a root directory"))? |
src/docker/remote.rs
Outdated
dirs.xargo(), | ||
reldst | ||
.rsplit_once('/') | ||
.ok_or_else(|| eyre::eyre!("cargo can't be a root directory"))? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.ok_or_else(|| eyre::eyre!("cargo can't be a root directory"))? | |
.ok_or_else(|| eyre::eyre!("can not be a root directory"))? |
bors try --none |
I want to add a test for this, but it's hard to imagine what a good test would look like. |
tryBuild succeeded: |
.cargo
and .xargo
correctlyCo-authored-by: josh-bpl <124796608+josh-bpl@users.noreply.github.com>
I think the test can be part of the larger testing infra work merging this as it has been confirmed to work now bors r+ |
Build failed: |
bors retry |
Build succeeded: |
resolves #1206