stty: Basic support for --file/-F

[VorpalBlade]

Fixes issue #3863

[VorpalBlade]

I would like to see the CI results on this. I'm not sure how to run the GNU test suite against this. But in my limited testing it works correctly, though the error messages are not the same on failure:

$ target/debug/coreutils stty -F /nosuch
stty: No such file or directory
$ stty -F /nosuch                       
stty: /nosuch: No such file or directory

$ target/debug/coreutils stty -F /bin     
thread 'main' panicked at 'Could not get terminal attributes: ENOTTY', src/uu/stty/src/stty.rs:171:44
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
$ stty -F /bin                       
stty: /bin: Inappropriate ioctl for device

There does also seem to be some other differences for stty, but I believe these are not related to -F support:

$ stty        
speed 38400 baud; line = 0;
-brkint -imaxbel iutf8
$ target/debug/coreutils stty        
speed 38400 baud; line = 0;
-brkint ixon -imaxbel iutf8 

$ stty -F /dev/pts/1
speed 38400 baud; line = 0;
erase = <undef>;
-brkint ixoff -imaxbel iutf8
$ target/debug/coreutils stty -F /dev/pts/1
speed 38400 baud; line = 0;
-brkint ixoff tandem ixon -imaxbel iutf8 

[sylvestre]

thanks
it would be nice to have a test in our implementation too :)

[VorpalBlade]

it would be nice to have a test in our implementation too :)

Agreed, though I'm not sure how to write a test of this! It might require creating a pseudo-terminal device just to test against.
Is there any existing infrastructure for that already? I didn't see anything in test_stty.rs

[VorpalBlade]

From the CI:

Error: ERROR: cargo fmt: style violation (file:'src/uu/stty/src/stty.rs', line:15; use cargo fmt -- "src/uu/stty/src/stty.rs")

I ran cargo fmt on that file locally and and that does not produce the diff the CI produces. I'm really confused now. I'm using rustfmt from the stable toolchain in rustup. Version is: rustfmt 1.5.1-stable (d5a82bb 2023-02-07).

What did I do wrong when formatting the file locally such that it decides on something else on the CI? Rustfmt is supposed to be stable between versions afaik also.

[tertsdiepraam]

Is there any existing infrastructure for that already? I didn't see anything in test_stty.rs

Nope, I was struggling with this when I wrote the first version of stty too. I'm open to suggestions!

I ran cargo fmt on that file locally and and that does not produce the diff the CI produces.

Reordering imports is a configuration feature, so maybe you rustfmt is not picking up on that configuration? It should be on by default: https://rust-lang.github.io/rustfmt/?version=v1.5.1&search=#reorder_imports

[tertsdiepraam]

Nice! I'd like to think about testing this better indeed. In the meantime, it looks good!

[VorpalBlade]

I believe the CI error is spurious. It is for a test unrelated to what I changed:

thread 'main' panicked at 'Error The system's UTC offset could not be determined retrieving the OffsetDateTime::now_local', tests/by-util/test_touch.rs:55:13

[VorpalBlade]

Nice! I'd like to think about testing this better indeed. In the meantime, it looks good!

Depending on how many other utilities in coreutils mess with the terminal, it might make sense to make a test harness that opens a pseudo-terminal. Most likely care will need to be taken if both the tester and the test framework are in the same thread to prevent deadlocks due to blocking IO. It might be easier to run it as a parent and child process.

That does not sound fun to implement however. How does GNU test this?

[tertsdiepraam]

Based on a quick loop at GNU, they just change what they can and reset it at the end. I like a solution with a pseudoterminal better :)

[VorpalBlade]

Spell checking failed on the comment due to referring to the flag "clocal". Not sure how to fix that, as "clocal" is mentioned in POSIX! Do you have a project specific dictionary somewhere?

[tertsdiepraam]

Yes, there's a custom dictionary in the .vscode folder and you add words to the file by putting clocal to the // spell-checker:ignore line at the top of the file.

[github-actions]

GNU testsuite comparison:

Congrats! The gnu test tests/misc/timeout is no longer failing!

[VorpalBlade]

This CI is so annoying. I don't even get what the issue is this time. Spurious/unrelated?

[tertsdiepraam]

This CI is so annoying. I don't even get what the issue is this time. Spurious/unrelated?

It's not unrelated and the suggestion in the error message fixes it:

Incompatible (or out-of-date) 'Cargo.lock' file; update using `cargo +1.64.0 update`

The Cargo.lock specifies a direct dependency on libc which is no longer needed if you access libc via nix. We know the CI is a lot, but it's necessary to have a project that

• has consistent styling and reasonable code quality,
• build on many platforms,
• keeps dependencies to a reasonable minimum,
• is guaranteed to build for a minimum Rust version.

The CI makes reviewing a lot easier and you'll probably get used to it very quickly. I also recommend checking out what commands the CI runs and running those locally before pushing.

[VorpalBlade]

The CI makes reviewing a lot easier and you'll probably get used to it very quickly. I also recommend checking out what commands the CI runs and running those locally before pushing.

Without doubt. However, the contributor experience for this could be better. I don't see a single easy script to run the CI checks locally (that would help, maybe something like cargo xtask). CICD.yml is exceedingly complicated, especially for someone who is not already an expert in GitHub Actions (which I'm not, I used Travis CI in the past, before Github Actions was a thing).

Since running the CI stuff locally appears quite complicated, what would help is if the turn around time from the CI was quicker! Waiting several hours for a maintainer to OK running the CI really isn't a good experience.

Because of all the friction contributing to this project, I suspect this will not just be the first time for me, but also the only time.

[sylvestre]

I agree. I would like to get some sponsors for this project to have faster builds...

[tertsdiepraam]

I don't see a single easy script to run the CI checks locally (that would help, maybe something like cargo xtask).

Good idea! We already have a Makefile.toml for cargo-make. We might be able to use that!

Waiting several hours for a maintainer to OK running the CI really isn't a good experience.

Btw, this only applies to first-time contributors and it's a GitHub policy that we can't disable sadly.

[VorpalBlade]

thread 'main' panicked at 'Error The system's UTC offset could not be determined retrieving the OffsetDateTime::now_local', tests/by-util/test_touch.rs:55:13

Issue #4253 and not related to my stuff.

Error: Timeout waiting for emulator to boot.

Spurious (Android)

ERROR: Unknown word (ugoa) (file:'src/uu/mkdir/mkdir.md', line:11)

Haven't touched that.

[tertsdiepraam]

No worries, we can recognize the stuff that's unrelated

[VorpalBlade]

Again, the windows error is unrelated to this PR (test_sort::test_separator_null), besides stty is not on windows.

The other error is also unrelated to this PR:
process didn't exit successfully: '/target/x86_64-unknown-linux-musl/debug/deps/uu_dd-fcb9efbecc5ab0ce' (signal: 11, SIGSEGV: invalid memory reference)

Is there anything else before this can be merged?

[tertsdiepraam]

We'll have to think about the tests separately, but I think this looks good for now! Thanks!