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.

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

fix #4767: chown -v 0 nf isn't showing a message #4768

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 15 additions & 4 deletions src/uu/chgrp/src/chgrp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
use uucore::display::Quotable;
pub use uucore::entries;
use uucore::error::{FromIo, UResult, USimpleError};
use uucore::perms::{chown_base, options, IfFrom};
use uucore::perms::{chown_base, options, GidUidOwnerFilter, IfFrom};
use uucore::{format_usage, help_about, help_usage};

use clap::{crate_version, Arg, ArgAction, ArgMatches, Command};
Expand All @@ -21,16 +21,22 @@ use std::os::unix::fs::MetadataExt;
static ABOUT: &str = help_about!("chgrp.md");
const USAGE: &str = help_usage!("chgrp.md");

fn parse_gid_and_uid(matches: &ArgMatches) -> UResult<(Option<u32>, Option<u32>, IfFrom)> {
fn parse_gid_and_uid(matches: &ArgMatches) -> UResult<GidUidOwnerFilter> {
let mut raw_group: String = String::new();
let dest_gid = if let Some(file) = matches.get_one::<String>(options::REFERENCE) {
fs::metadata(file)
.map(|meta| Some(meta.gid()))
.map(|meta| {
let gid = meta.gid();
raw_group = entries::gid2grp(gid).unwrap_or_else(|_| gid.to_string());
Some(gid)
})
.map_err_context(|| format!("failed to get attributes of {}", file.quote()))?
} else {
let group = matches
.get_one::<String>(options::ARG_GROUP)
.map(|s| s.as_str())
.unwrap_or_default();
raw_group = group.to_string();
if group.is_empty() {
None
} else {
Expand All @@ -45,7 +51,12 @@ fn parse_gid_and_uid(matches: &ArgMatches) -> UResult<(Option<u32>, Option<u32>,
}
}
};
Ok((dest_gid, None, IfFrom::All))
Ok(GidUidOwnerFilter {
dest_gid,
dest_uid: None,
raw_owner: raw_group,
filter: IfFrom::All,
})
}

#[uucore::main]
Expand Down
29 changes: 23 additions & 6 deletions src/uu/chown/src/chown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

use uucore::display::Quotable;
pub use uucore::entries::{self, Group, Locate, Passwd};
use uucore::perms::{chown_base, options, IfFrom};
use uucore::perms::{chown_base, options, GidUidOwnerFilter, IfFrom};
use uucore::{format_usage, help_about, help_usage};

use uucore::error::{FromIo, UResult, USimpleError};
Expand All @@ -23,7 +23,7 @@ static ABOUT: &str = help_about!("chown.md");

const USAGE: &str = help_usage!("chown.md");

fn parse_gid_uid_and_filter(matches: &ArgMatches) -> UResult<(Option<u32>, Option<u32>, IfFrom)> {
fn parse_gid_uid_and_filter(matches: &ArgMatches) -> UResult<GidUidOwnerFilter> {
let filter = if let Some(spec) = matches.get_one::<String>(options::FROM) {
match parse_spec(spec, ':')? {
(Some(uid), None) => IfFrom::User(uid),
Expand All @@ -37,17 +37,34 @@ fn parse_gid_uid_and_filter(matches: &ArgMatches) -> UResult<(Option<u32>, Optio

let dest_uid: Option<u32>;
let dest_gid: Option<u32>;
let raw_owner: String;
if let Some(file) = matches.get_one::<String>(options::REFERENCE) {
let meta = fs::metadata(file)
.map_err_context(|| format!("failed to get attributes of {}", file.quote()))?;
dest_gid = Some(meta.gid());
dest_uid = Some(meta.uid());
let gid = meta.gid();
let uid = meta.uid();
dest_gid = Some(gid);
dest_uid = Some(uid);
raw_owner = format!(
"{}:{}",
entries::uid2usr(uid).unwrap_or_else(|_| uid.to_string()),
entries::gid2grp(gid).unwrap_or_else(|_| gid.to_string())
);
} else {
let (u, g) = parse_spec(matches.get_one::<String>(options::ARG_OWNER).unwrap(), ':')?;
raw_owner = matches
.get_one::<String>(options::ARG_OWNER)
.unwrap()
.into();
let (u, g) = parse_spec(&raw_owner, ':')?;
dest_uid = u;
dest_gid = g;
}
Ok((dest_gid, dest_uid, filter))
Ok(GidUidOwnerFilter {
dest_gid,
dest_uid,
raw_owner,
filter,
})
}

#[uucore::main]
Expand Down
30 changes: 26 additions & 4 deletions src/uucore/src/lib/features/perms.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ pub enum TraverseSymlinks {
pub struct ChownExecutor {
pub dest_uid: Option<u32>,
pub dest_gid: Option<u32>,
pub raw_owner: String, // The owner of the file as input by the user in the command line.
pub traverse_symlinks: TraverseSymlinks,
pub verbosity: Verbosity,
pub filter: IfFrom,
Expand All @@ -207,7 +208,16 @@ impl ChownExecutor {
let path = root.as_ref();
let meta = match self.obtain_meta(path, self.dereference) {
Some(m) => m,
_ => return 1,
_ => {
if self.verbosity.level == VerbosityLevel::Verbose {
println!(
"failed to change ownership of {} to {}",
path.quote(),
self.raw_owner
);
}
return 1;
}
};

// Prohibit only if:
Expand Down Expand Up @@ -412,7 +422,13 @@ pub mod options {
pub const ARG_FILES: &str = "FILE";
}

type GidUidFilterParser = fn(&ArgMatches) -> UResult<(Option<u32>, Option<u32>, IfFrom)>;
pub struct GidUidOwnerFilter {
pub dest_gid: Option<u32>,
pub dest_uid: Option<u32>,
pub raw_owner: String,
pub filter: IfFrom,
}
type GidUidFilterOwnerParser = fn(&ArgMatches) -> UResult<GidUidOwnerFilter>;

/// Base implementation for `chgrp` and `chown`.
///
Expand All @@ -425,7 +441,7 @@ pub fn chown_base(
mut command: Command,
args: impl crate::Args,
add_arg_if_not_reference: &'static str,
parse_gid_uid_and_filter: GidUidFilterParser,
parse_gid_uid_and_filter: GidUidFilterOwnerParser,
groups_only: bool,
) -> UResult<()> {
let args: Vec<_> = args.collect();
Expand Down Expand Up @@ -508,12 +524,18 @@ pub fn chown_base(
} else {
VerbosityLevel::Normal
};
let (dest_gid, dest_uid, filter) = parse_gid_uid_and_filter(&matches)?;
let GidUidOwnerFilter {
dest_gid,
dest_uid,
raw_owner,
filter,
} = parse_gid_uid_and_filter(&matches)?;

let executor = ChownExecutor {
traverse_symlinks,
dest_gid,
dest_uid,
raw_owner,
verbosity: Verbosity {
groups_only,
level: verbosity_level,
Expand Down
12 changes: 6 additions & 6 deletions tests/by-util/test_chown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -730,15 +730,15 @@ fn test_chown_file_notexisting() {
let user_name = String::from(result.stdout_str().trim());
assert!(!user_name.is_empty());

let _result = scene
scene
.ucmd()
.arg(user_name)
.arg(&user_name)
.arg("--verbose")
.arg("not_existing")
.fails();

// TODO: uncomment once "failed to change ownership of '{}' to {}" added to stdout
// result.stderr_contains("retained as");
.fails()
.stdout_contains(format!(
"failed to change ownership of 'not_existing' to {user_name}"
));
// TODO: uncomment once message changed from "cannot dereference" to "cannot access"
// result.stderr_contains("cannot access 'not_existing': No such file or directory");
}