-
Notifications
You must be signed in to change notification settings - Fork 35
Implement namei command #278
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
base: main
Are you sure you want to change the base?
Conversation
Can you please run |
9fc8fda
to
66fe3b6
Compare
Removed two unnecessary lines
I have run |
I have added tests. Will add windows support soon. |
Implemented windows support. All checks are passing. Maintainers @cakebaker , please review and merge, if there are no issues. |
src/uu/namei/Cargo.toml
Outdated
[dependencies] | ||
uucore = { workspace = true } | ||
clap = { workspace = true } | ||
selinux = { version = "0.1", optional = true } |
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.
What's the reason for this old version of the selinux
crate? The current version is 0.5.1
.
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.
That is a typo. I have fixed it.
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.
Can you please also commit the updated Cargo.lock
file?
#[test] | ||
fn test_fails_on_non_existing_path() { | ||
let (at, mut ucmd) = at_and_ucmd!(); | ||
let argmnt = at.plus_as_string("nonexisting"); | ||
|
||
#[cfg(target_os = "windows")] | ||
let err = "The system cannot find the file specified"; | ||
#[cfg(not(target_os = "windows"))] | ||
let err = "No such file or directory"; | ||
|
||
ucmd.arg(argmnt).fails().code_is(1).stderr_contains(err); | ||
} |
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.
It's great that you test what happens if a path doesn't exist. What happens in the other case, if a path does exist?
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.
If path does exist then the code runs as expected. Tests for such cases are given in test_long_arg test_mode_arg etc.
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.
If path does exist then the code runs as expected
Are you sure? ;-)
I get the following output:
$ cargo run -q namei README.md
f: README.md
/ - No such file or directory (os error 2)
$ echo $?
1
wheras with the original namei
I get:
$ namei README.md
f: README.md
- README.md
$ echo $?
0
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.
I think somewhere i haven't handled absolute vs relative filepaths properly. I will fix it.
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.
Hello @cakebaker it should work now.
Implemented namei command without selinux support. Will add that soon...