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

date: fix interaction of flags, fix issues around --set #6142

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

BenWiederhake
Copy link
Collaborator

This PR fixes multiple issues around multi-flags:

  • Many repeated flags used to be handled incorrectly; this is work toward Allow repeated flags much more often #5998
  • We used to accept the --reference argument, but completely ignored it. Yikes!
  • Input, parsing, and output of --set was completely broken. This fixes it largely.
  • Input options (-d, -f, -r), formatting options (-I, -R, --rfc-3339), and --set are often mutually-exclusive, with the exception that input options override themselves (but not each other), and also --set can be combined with formatting options.

Some things are unfixable, in particular the order-sensitive behavior; see #4254#issuecomment-2026446634 and uutils/uutils-args#113

Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)

@BenWiederhake
Copy link
Collaborator Author

"CICD / Build (macos-latest, x86_64-apple-darwin, feat_os_macos)" flaked:

---- test_stat::test_stdin_pipe_fifo1 stdout ----
run: /Users/runner/work/coreutils/coreutils/target/x86_64-apple-darwin/debug/coreutils stat -
run: /Users/runner/work/coreutils/coreutils/target/x86_64-apple-darwin/debug/coreutils stat -L -
thread 'test_stat::test_stdin_pipe_fifo1' panicked at ''  File: -
  Size: 0         	Blocks: 0          IO Block: 512    weird file
Device: 25d29ea1h/634560161d	Inode: 2178033826  Links: 0
Access: (0660/?rw-rw----)  Uid: (  501/  runner)   Gid: (   20/   staff)
Access: 2024-03-29 16:28:42.942290340 +0000
Modify: 2024-03-29 16:28:42.942290340 +0000
Change: 2024-03-29 16:28:42.942290340 +0000
 Birth: 1970-01-01 00:00:00.000000000 +0000
' does not contain 'fifo'', tests/by-util/test_stat.rs:286:10

All other failures appear to be known issues, too.

@BenWiederhake
Copy link
Collaborator Author

Changes since last push:

@BenWiederhake
Copy link
Collaborator Author

Changes since last push:

  • While resolving the conflict, I accidentally dropped the #[test] marker for a test. Whoops! Added it back in.

@BenWiederhake
Copy link
Collaborator Author

Changes since last push:

Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/inotify-dir-recreate (passes in this run but fails in the 'main' branch)

@BenWiederhake
Copy link
Collaborator Author

BenWiederhake commented Apr 6, 2024

Changes since last push: Nothing, just a rebase to show that it's still a good idea to do this.

Copy link

github-actions bot commented Apr 6, 2024

GNU testsuite comparison:

Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)

@BenWiederhake
Copy link
Collaborator Author

CI failures seem to be only flakes:

Example:

---- test_pinky::test_short_format_q stdout ----
run: /Users/runner/work/coreutils/coreutils/target/x86_64-apple-darwin/debug/coreutils pinky -q
thread 'test_pinky::test_short_format_q' panicked at 'Command was expected to succeed. code: 101
stdout = Login     TTY      When            
runner   *console 
 stderr = thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: IndeterminateOffset', src/uucore/src/lib/features/utmpx.rs:192:62
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
', tests/by-util/test_pinky.rs:97:40

@BenWiederhake
Copy link
Collaborator Author

BenWiederhake commented Jul 3, 2024

Changes since last push:

  • Rebased to new main
  • Nothing more, I just want to remind that this PR exists and is still a good idea.

What else needs to be done?
EDIT: Those CI failures look suspicious, I'll have a look.

@sylvestre sylvestre force-pushed the dev-date-repeated branch 2 times, most recently from aa5120d to 5264cb8 Compare July 9, 2024 21:13
};

if let Some(date) = settings.set_to {
// Iterate over all dates - whether it's a single date or a file.
let dates: Box<dyn Iterator<Item = _>> = if let Some(date_string) = &settings.set_to {
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

it is starting to be a long function.
maybe split it ?

.replace("%f", "%N");
println!("{formatted}");
// Format all the dates
for date in dates {
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

like can the content of this loop be in a function ?

vec!["-f", "foo", "-r", "foo"],
// Format with other format
vec!["-I", "-R"],
vec!["-I", "--rfc-3339=date"],
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

nice

@BenWiederhake
Copy link
Collaborator Author

Changes since last push:

  • Rebase on current main (that's around 100 commits!)
  • Split the gargantuan uumain function into four separate functions, as requested, and remove the corresponding clippy lint.

The only CI failure is #6534, as usual.

Copy link

GNU testsuite comparison:

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

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.

2 participants