search

uutils / coreutils

Cross-platform Rust rewrite of the GNU coreutils
https://uutils.github.io/
MIT License
17.86k stars 1.29k forks source link

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

Open TheDcoder opened 1 year ago

TheDcoder commented 1 year ago

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 commented 1 year ago

Found in the wild here:

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

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 commented 1 year ago

Wondering if this (moving cp code to uucore) could be a good "first step" for https://github.com/uutils/coreutils/issues/5203 ?

Javatrix commented 3 months ago

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

TheDcoder commented 3 months ago

What fixed the issue? :thinking:

Javatrix commented 3 months ago

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

DaringCuteSeal commented 3 weeks ago

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 commented 3 weeks ago

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

DaringCuteSeal commented 3 weeks ago

i'll try

DaringCuteSeal commented 3 weeks ago

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