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

Match GNU semantics for missing EOF #4009

Merged
merged 1 commit into from
Oct 17, 2022

Conversation

andrewbaptist
Copy link
Contributor

While the rust coreutils semantics were arguably more correct, they were different than the gnu split semantics when handling a file without a trailing EOF. This patch addresses that difference and allows passing one more GNU test suite.

While the rust coreutils semantics were arguably more correct,
they were different than the gnu split semantics when handling a
file without a trailing EOF. This patch addresses that difference
and allows passing one more GNU test suite.
@tertsdiepraam
Copy link
Member

tertsdiepraam commented Oct 8, 2022

Edited comment: I was extremely wrong before, so I just deleted it all.

@@ -659,6 +659,22 @@ fn test_line_bytes_no_empty_file() {
assert!(!at.plus("xak").exists());
}

#[test]
Copy link
Contributor

Choose a reason for hiding this comment

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

I am confused why this isn't testing the "-io" option too ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ---io option is unspecified behavior for GNU split. The code will currently accept and ignore any ---io flags. It is not clear how the flag should be handled as the "chunking" of data is very different.

@sylvestre
Copy link
Contributor

Well done for:
Warning: Congrats! The gnu test tests/split/line-bytes is no longer failing!

@andrewbaptist
Copy link
Contributor Author

Edited comment: I was extremely wrong before, so I just deleted it all.

No problem - this was extremely confusing to me why the GNU split does this. As I mentioned in the comment, the previous Rust behavior was superior and more correct than what I had to change it to. I'm guessing this isn't the only case. It seems like we should have some type of compile-time variable which is "MATCH_GNU" that is enabled for matching GNU behavior but in cases where the behavior is "bad" we don't enable it in "release" builds, or maybe do today, but once we get to 100% matching and the usage of these tools pick up we can find and change all the places. I don't expect anyone is going to be affected by this change in behavior.

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.

3 participants