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

install: Accept special files like /dev/stdin #5080

Closed
TheDcoder opened this issue Jul 13, 2023 · 11 comments
Closed

install: Accept special files like /dev/stdin #5080

TheDcoder opened this issue Jul 13, 2023 · 11 comments

Comments

@TheDcoder
Copy link
Contributor

GNU coreutils accepts special files like /dev/stdin without any issues, for e.g.:

$ echo 'foo' | install /dev/stdin test

That creates a file called test with foo\n as its contents, which isn't surprising.

However our coreutils fail with the same usage with this error:

install: cannot install '/dev/stdin' to 'test': Invalid input parameter

This issue doesn't effect cp, however it might effect other utils which haven't been tested.

@TheDcoder
Copy link
Contributor Author

Found in the wild here:

echo 'g keyd' | install -Dm644 /dev/stdin "${pkgdir}/usr/lib/sysusers.d/${pkgname%-git}.conf"

@tertsdiepraam
Copy link
Member

I checked the source code and install just uses std::fs::copy, which indeed doesn't handle that case. cp has a more complicated procedure which falls back to copying the contents. I think it might make sense to extract that procedure to uucore and share it between the two utils. Could also be fun to make it a separate crate later too, because it might be useful for other projects to have a more flexible copy function, but let's start with uucore.

@granquet
Copy link
Contributor

Wondering if this (moving cp code to uucore) could be a good "first step" for #5203 ?

@Javatrix
Copy link

Checked right now, works correctly. Perhaps the issue should be closed?

@TheDcoder
Copy link
Contributor Author

What fixed the issue? 🤔

@Javatrix
Copy link

I don't know, but this issue seems to be quite old. Perhaps somebody fixed it and forgot to mark this as complete.

@DaringCuteSeal
Copy link
Contributor

Issue still persists apparently. The only time when it works is when I use zsh's herestring which works because zsh creates some sort of non-special temporary file I think. Piping to the command still breaks install though. I think /dev/stdin should be handled specially, kinda like how uutil's cat does it

@sylvestre
Copy link
Contributor

Don't hesitate to provide a patch, it should not be hard

@DaringCuteSeal
Copy link
Contributor

i'll try

@DaringCuteSeal
Copy link
Contributor

DaringCuteSeal commented Nov 17, 2024

I checked the source code and install just uses std::fs::copy, which indeed doesn't handle that case. cp has a more complicated procedure which falls back to copying the contents. I think it might make sense to extract that procedure to uucore and share it between the two utils. Could also be fun to make it a separate crate later too, because it might be useful for other projects to have a more flexible copy function, but let's start with uucore.

since this also affects cp i figured that i may wanna do exactly that

@cakebaker
Copy link
Contributor

Fixed in #6965

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants