Skip to content
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.

Sign up for GitHub

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

Jump to bottom

cp: show mode if target does not have S_IWUSR #6696

Closed
wants to merge 8 commits into from
Closed

cp: show mode if target does not have S_IWUSR #6696

Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e2a8e86
cp: show mode if target does not have S_IWUSR
andrewliebenow Sep 15, 2024
5414039
cp: with -i, delete destination if needed
andrewliebenow Sep 15, 2024
e3e3b62
Move code into separate functions
andrewliebenow Sep 15, 2024
a254f5e
Add missing cast
andrewliebenow Sep 15, 2024
6aaaeae
Fix unused variable lint
andrewliebenow Sep 15, 2024
5337aec
Replace "expect" with non-experimental "allow"
andrewliebenow Sep 15, 2024
dd9cb2e
Update src/uu/cp/src/cp.rs
andrewliebenow Sep 16, 2024
55f0cbc
Merge branch 'main' into fix-issue-6658
andrewliebenow Sep 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 90 additions & 37 deletions src/uu/cp/src/cp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1410,7 +1410,56 @@
Err(Error::Skipped(false))
}
Self::Interactive(_) => {
if prompt_yes!("overwrite {}?", path.quote()) {
// TODO
// The destination metadata can be read multiple times in the course of a single execution of `cp`.
// This fix adds yet another metadata read.
// Should this metadata be read once and then reused throughout the execution?
// https://github.com/uutils/coreutils/issues/6658
let extra_prompt_information: Option<(String, String)> = {
#[cfg(unix)]
andrewliebenow marked this conversation as resolved.
Show resolved Hide resolved
{
use libc::{mode_t, S_IWUSR};
use std::os::unix::prelude::MetadataExt;

match path.metadata() {
Ok(me) => {
let mode: mode_t = me.mode();

// It looks like this extra information is added to the prompt iff the file's user write bit is 0
if uucore::has!(mode, S_IWUSR) {
None
} else {
// Discard leading digits
let mode_without_leading_digits = mode & 0o7777;

Some((
format!("{mode_without_leading_digits:04o}"),
uucore::fs::display_permissions_unix(mode, false),
))
}
}
// TODO: How should failure to read the metadata be handled? Ignoring for now.
Err(_) => None,

Check warning on line 1442 in src/uu/cp/src/cp.rs

View check run for this annotation

Codecov / codecov/patch

src/uu/cp/src/cp.rs#L1442 

Added line #L1442 was not covered by tests
}
}

#[cfg(not(unix))]
{
None
}
};

let prompt_yes_result =
if let Some((octal, human_readable)) = extra_prompt_information {
prompt_yes!(
"replace {}, overriding mode {octal} ({human_readable})?",
path.quote()
)
} else {
prompt_yes!("overwrite {}?", path.quote())
};

if prompt_yes_result {
Ok(())
} else {
Err(Error::Skipped(true))
Expand Down Expand Up @@ -1680,44 +1729,48 @@
}
if !is_dest_removed {
match options.overwrite {
// FIXME: print that the file was removed if --verbose is enabled
OverwriteMode::Clobber(ClobberMode::Force) => {
if is_symlink_loop(dest) || fs::metadata(dest)?.permissions().readonly() {
fs::remove_file(dest)?;
}
}
OverwriteMode::Clobber(ClobberMode::RemoveDestination) => {
fs::remove_file(dest)?;
}
OverwriteMode::Clobber(ClobberMode::Standard) => {
// Consider the following files:
//
// * `src/f` - a regular file
// * `src/link` - a hard link to `src/f`
// * `dest/src/f` - a different regular file
//
// In this scenario, if we do `cp -a src/ dest/`, it is
// possible that the order of traversal causes `src/link`
// to get copied first (to `dest/src/link`). In that case,
// in order to make sure `dest/src/link` is a hard link to
// `dest/src/f` and `dest/src/f` has the contents of
// `src/f`, we delete the existing file to allow the hard
// linking.

if options.preserve_hard_links()
// only try to remove dest file only if the current source
// is hardlink to a file that is already copied
&& copied_files.contains_key(
&FileInformation::from_path(
source,
options.dereference(source_in_command_line),
)
.context(format!("cannot stat {}", source.quote()))?,
) {
fs::remove_file(dest)?;
OverwriteMode::Clobber(cl) | OverwriteMode::Interactive(cl) => {
andrewliebenow marked this conversation as resolved.
Show resolved Hide resolved
match cl {
// FIXME: print that the file was removed if --verbose is enabled
ClobberMode::Force => {
if is_symlink_loop(dest) || fs::metadata(dest)?.permissions().readonly() {
fs::remove_file(dest)?;
}
}
ClobberMode::RemoveDestination => {
fs::remove_file(dest)?;
}
ClobberMode::Standard => {
// Consider the following files:
//
// * `src/f` - a regular file
// * `src/link` - a hard link to `src/f`
// * `dest/src/f` - a different regular file
//
// In this scenario, if we do `cp -a src/ dest/`, it is
// possible that the order of traversal causes `src/link`
// to get copied first (to `dest/src/link`). In that case,
// in order to make sure `dest/src/link` is a hard link to
// `dest/src/f` and `dest/src/f` has the contents of
// `src/f`, we delete the existing file to allow the hard
// linking.

if options.preserve_hard_links() &&
// only try to remove dest file only if the current source
// is hardlink to a file that is already copied
copied_files.contains_key(
&FileInformation::from_path(
source,
options.dereference(source_in_command_line)
).context(format!("cannot stat {}", source.quote()))?
)
{
fs::remove_file(dest)?;
}
}
}
}
_ => (),
OverwriteMode::NoClobber => {}
};
}

Expand Down
Loading