Skip to content

Conversation

@ChrisDryden
Copy link
Collaborator

@ChrisDryden ChrisDryden commented Dec 29, 2025

This is the third PR based off of the following prototype: #9866

Now the original SMACK implementation for ls is implemented alongside the CI runner for the SMACK GNU tests and now its time to actually implement SMACK support for the different utilities.

@ChrisDryden ChrisDryden force-pushed the smack-utility-support branch 2 times, most recently from d1808f0 to 070b999 Compare December 29, 2025 16:31
if state.cflag {
return if state.selinux_supported {
// print SElinux context and exit
#[cfg(all(any(target_os = "linux", target_os = "android"), feature = "selinux"))]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is unrelated to the SMACK fixes, but I am unsure as to why we need this linux and android flag here if we are targeting the SELINUX feature

@ChrisDryden ChrisDryden force-pushed the smack-utility-support branch 2 times, most recently from a6003eb to f2fb5e4 Compare December 29, 2025 16:43
@codspeed-hq
Copy link

codspeed-hq bot commented Dec 30, 2025

CodSpeed Performance Report

Merging #9910 will degrade performance by 5.89%

Comparing ChrisDryden:smack-utility-support (d482e41) with main (1eea517)

Summary

❌ 10 regressions
✅ 126 untouched
⏩ 24 skipped1

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

Benchmarks breakdown

Benchmark BASE HEAD Efficiency
wc_bytes_synthetic[500] 185.2 µs 191.7 µs -3.37%
dd_copy_partial 550.8 µs 571.7 µs -3.65%
factor_multiple_u64s[2] 212.3 ms 225.6 ms -5.89%
sort_ascii_utf8_locale 22.2 ms 23.6 ms -5.78%
b64_encode_synthetic 161.9 µs 169.4 µs -4.44%
b64_decode_synthetic 166.3 µs 171.8 µs -3.19%
b64_decode_ignore_garbage_synthetic 163.2 µs 169.7 µs -3.81%
rm_single_file 119.3 ms 124.3 ms -4%
mv_single_file 135.5 ms 141.4 ms -4.13%
mv_force_overwrite 144.3 ms 150 ms -3.79%

Footnotes

  1. 24 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.

@ChrisDryden ChrisDryden force-pushed the smack-utility-support branch from f2fb5e4 to 05fd505 Compare December 30, 2025 21:43
@ChrisDryden ChrisDryden marked this pull request as ready for review December 30, 2025 22:37
@ChrisDryden
Copy link
Collaborator Author

I'm not sure why the GNU comment is not posting?

I'm seeing this in the workflow but it didn't post the comment
Notice: Congrats! The gnu test tests/id/smack is no longer failing!
Notice: Congrats! The gnu test tests/mkdir/smack-no-root is no longer failing!
Notice: Congrats! The gnu test tests/mkdir/smack-root is no longer failing!

@sylvestre
Copy link
Contributor

i can take some time

also, please fix:

  error: redundant else block
     --> src/uu/id/src/id.rs:186:14
      |
  186 |               } else {
      |  ______________^
  187 | |                 return Err(USimpleError::new(
  188 | |                     1,
  189 | |                     translate!("id-error-cannot-get-context"),
  190 | |                 ));
  191 | |             }
      | |_____________^
      |
      = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.92.0/index.html#redundant_else
      = note: `-D clippy::redundant-else` implied by `-D warnings`
      = help: to override `-D warnings` add `#[allow(clippy::redundant_else)]`
  help: remove the `else` block and move the contents out
      |
  186 ~             }
  187 +             return Err(USimpleError::new(
  188 +                 1,
  189 +                 translate!("id-error-cannot-get-context"),
  190 +             ));
      |

@ChrisDryden
Copy link
Collaborator Author

As a note about the performance regressions, its from the default locale file growing bigger. This can be improved by duplicating it to the individual utility locales, could go either way, just leaving it in the main locale file for now.

@ChrisDryden ChrisDryden force-pushed the smack-utility-support branch from 05fd505 to a34b2f0 Compare December 30, 2025 22:44
@sylvestre
Copy link
Contributor

As a note about the performance regressions, its from the default locale file growing bigger. This can be improved by duplicating it to the individual utility locales, could go either way, just leaving it in the main locale file for now.

We should work on this at some point, we should not have such perf regressions for a few strings

@github-actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/id/smack is no longer failing!
Congrats! The gnu test tests/mkdir/smack-no-root is no longer failing!
Congrats! The gnu test tests/mkdir/smack-root is no longer failing!

@ChrisDryden
Copy link
Collaborator Author

Created an issue to track the shared locale files impacting performance: #9932 I think its a combination of gating the locale files based on feature flag and also coming up with mechanisms to reduce the burden that locales have on performance

@github-actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/id/smack is no longer failing!
Congrats! The gnu test tests/mkdir/smack-no-root is no longer failing!
Congrats! The gnu test tests/mkdir/smack-root is no longer failing!

@github-actions
Copy link

GNU testsuite comparison:

Congrats! The gnu test tests/id/smack is no longer failing!
Congrats! The gnu test tests/mkdir/smack-no-root is no longer failing!
Congrats! The gnu test tests/mkdir/smack-root 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