Rm no abbreviation no preserve root

[cerdelen]

Fix: #10188

Added a runtime check that --no-preserve-root is literal and was not abbreviated.

regression tests are included.

[github-actions]

GNU testsuite comparison:

Skipping an intermittent issue tests/shuf/shuf-reservoir (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/sort/sort-stale-thread-mem (passes in this run but fails in the 'main' branch)

[codspeed-hq]

Merging this PR will degrade performance by 17.36%

⚡ 7 improved benchmarks
❌ 5 regressed benchmarks
✅ 270 untouched benchmarks
⏩ 38 skipped benchmarks¹

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Mode	Benchmark	BASE	HEAD	Efficiency
❌	Simulation	rm_multiple_files	2.1 ms	2.3 ms	-8.35%
❌	Simulation	rm_force_files	2 ms	2.2 ms	-9.36%
⚡	Memory	sort_unique_locale[500000]	36.9 MB	33 MB	+11.83%
⚡	Memory	sort_mixed_data[500000]	25.8 MB	22.4 MB	+15.21%
⚡	Memory	sort_numeric[500000]	48.7 MB	46 MB	+5.82%
⚡	Memory	sort_ascii_only[500000]	25.5 MB	21.9 MB	+16.75%
⚡	Memory	sort_accented_data[500000]	25.5 MB	21.6 MB	+18.11%
⚡	Memory	sort_key_field[500000]	32.8 MB	29.2 MB	+12.53%
❌	Memory	sort_long_line[160000]	716.7 KB	858.6 KB	-16.53%
❌	Memory	rm_force_files	293.1 KB	354.7 KB	-17.36%
❌	Memory	rm_multiple_files	304.7 KB	355.2 KB	-14.21%
⚡	Memory	du_wide_tree[(5000, 500)]	1.3 MB	1.2 MB	+4%

---

_{Comparing cerdelen:rm_no_abbreviation_no_preserve_root (b40ced8) with main (d7ed3c6)}

Footnotes

1. 38 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[github-actions]

GNU testsuite comparison:

Skipping an intermittent issue tests/shuf/shuf-reservoir (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/sort/sort-stale-thread-mem (passes in this run but fails in the 'main' branch)

[anastygnome]

Hmm
Maybe you could do it this way :
Add a --no-preserve-roo (yes , without the t) explicit arg to the clap parser. Check if it matches, if it does, reject. This way you don't have to parse all args :)

[cerdelen]

This way i would produce more overhead at the parsing stage though that will be run everytime, while the way it is right now the iteration over all args only occurs if at the parsing stage it was determined no preserve mode was active, right?

Also using matches.get_flag() also is just an iteration over all "valid args" as far as i see, so introducing the --no-preserve-roo (without t) makes the clap get_matches have a bigger overhead and doesn't save any runtime cost later on.

Please tell me if i have somewhere a wrong understanding how clap works.

[anastygnome]

No, because the parser is already trying to match no preserve root, so clap will optimize the parsing. , you're already adding overhead with trying to get the match every time.
You should create a new arg as in m'y above message with a value parser that always returns a Err

[cerdelen]

As soon as i add another arg as you proposed clap starts enforcing the correct spelling but the error Messages are the long ones from clap itself.

`❯ ./target/debug/rm --no-pre l
error: unexpected argument '--no-pre' found

tip: a similar argument exists: '--no-preserve-roo'

Usage: rm [OPTION]... FILE...

For more information, try '--help'.
`

Also trying to use a value_parser that always returns an Error produces a different error Message by clap

`❯ ./target/debug/rm --no-pre l
error: invalid value 'l' for '--no-preserve-roo ': you may not abbreviate the --no-preserve-root option

For more information, try '--help'.`

So if we want a clean GNU style error message of just 'you may not abbreviate the --no-preserve-root option' i think we have to parse this manually apart from the clap library unfortunately.

[anastygnome]

Oh, okay. if that's the case, you should add the no-preserve-roo flag and make the check after parsing. If it is present, just throw an error. That way we get most performance

[github-actions]

GNU testsuite comparison:

Skipping an intermittent issue tests/tty/tty-eof (passes in this run but fails in the 'main' branch)

[github-actions]

GNU testsuite comparison:

Skipping an intermittent issue tests/shuf/shuf-reservoir (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/sort/sort-stale-thread-mem (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/ptx/ptx-overrun is no longer failing!

[cerdelen]

@sylvestre the failing Android test seems to be failing for all other PR's too (and on main too). So it looks to me not to be caused by my changes for this PR

[sylvestre]

yeah, just ignore it :)