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

stat: Adds support to read a filename redirected to stdin #3280

Conversation

crazystylus
Copy link
Contributor

@crazystylus crazystylus commented Mar 20, 2022

Explanation:

touch f && stat - < f creates an empty file and stat after seeing - tries to find the file which is being redirected to stdin and prints details for that file.

Solution:

On detecting -, canonicalize /dev/stdin which will point to the file which is being redirected to stdin

NOTE stat doesn't read the content of the file, and this can be verified

Fixes #3241

- Canonicalization of `/dev/stdin` which points to stdin file
@sylvestre
Copy link
Sponsor Contributor

Could you please add a test to make sure we don't regress?
thanks

Now all instances of `-` will be replaced with real / canonicalized
path of `/dev/stdin`
@crazystylus
Copy link
Contributor Author

Hi,
It's not possible to add test cases for this at the moment because when I attach a file handle to process's stdin for redirection in std::process::Command or UCommand, rust will instead attach a pipe. std::fs::canonicalize is failing to resolve /dev/stdin when it points to a linux pipe at the moment.

Reference: rust-lang/rust#95239

Copy link
Collaborator

@jfinkels jfinkels left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though there is no simple way to test this, I checked out this branch and tested on my machine and it seemed to work as expected. 👍

@sylvestre sylvestre merged commit 53baeca into uutils:main Apr 9, 2022
@sylvestre
Copy link
Sponsor Contributor

Thanks!

@crazystylus crazystylus deleted the stat-fails-to-read-a-file-redirected-to-stdin branch April 9, 2022 07:23
jhscheer added a commit to jhscheer/coreutils that referenced this pull request May 5, 2022
jhscheer added a commit to jhscheer/coreutils that referenced this pull request May 5, 2022
jhscheer added a commit to jhscheer/coreutils that referenced this pull request May 6, 2022
* fix uutils#3485
* improve the workaround from uutils#3280
* add tests
@jhscheer
Copy link
Contributor

jhscheer commented May 6, 2022

Hi, It's not possible to add test cases for this at the moment because when I attach a file handle to process's stdin for redirection in std::process::Command or UCommand, rust will instead attach a pipe. std::fs::canonicalize is failing to resolve /dev/stdin when it points to a linux pipe at the moment.

Reference: rust-lang/rust#95239

It was a bit tricky, but it can be tested like this:

// $ touch f && stat - < f
// File: -
// Size: 0 Blocks: 0 IO Block: 4096 regular empty file
let ts = TestScenario::new(util_name!());
let at = &ts.fixtures;
at.touch("f");
new_ucmd!()
.arg("-")
.set_stdin(std::fs::File::open("f").unwrap())
.run()
.no_stderr()
.stdout_contains("regular empty file")
.stdout_contains("File: -")
.succeeded();

jhscheer added a commit to jhscheer/coreutils that referenced this pull request May 6, 2022
* fix uutils#3485
* improve the workaround from uutils#3280
* add tests
jhscheer added a commit to jhscheer/coreutils that referenced this pull request May 6, 2022
* fix uutils#3485
* improve the workaround from uutils#3280
* add tests
jhscheer added a commit to jhscheer/coreutils that referenced this pull request May 6, 2022
* fix uutils#3485
* improve the workaround from uutils#3280
* add tests
jhscheer added a commit to jhscheer/coreutils that referenced this pull request May 6, 2022
* fix uutils#3485
* improve the workaround from uutils#3280
* add tests
jhscheer added a commit to jhscheer/coreutils that referenced this pull request May 7, 2022
* fix uutils#3485
* improve the workaround from uutils#3280
* add tests
sylvestre pushed a commit to jhscheer/coreutils that referenced this pull request May 11, 2022
* fix uutils#3485
* improve the workaround from uutils#3280
* add tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

stat: fails to read a file redirected to stdin
4 participants