-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
ls: Add SMACK support #9868
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
ls: Add SMACK support #9868
Conversation
1495149 to
3a835f9
Compare
CodSpeed Performance ReportMerging #9868 will not alter performanceComparing Summary
Footnotes
|
|
The performance regressions come from adding things to the fluent bundle. Looks like its better to move the translations to much more specific paths. |
|
GNU testsuite comparison: |
src/uucore/src/lib/features/smack.rs
Outdated
|
|
||
| /// Checks if SMACK is enabled by verifying smackfs is mounted. | ||
| pub fn is_smack_enabled() -> bool { | ||
| Path::new("/sys/fs/smackfs").exists() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the result should be cached
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Want me to do the same for SELinux? I double checked it before adding a OnceCell, but can add this easily
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just added it for smack, can follow up with SELinux in another PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, thanks! :)
|
GNU testsuite comparison: |
Co-authored-by: Sylvestre Ledru <[email protected]>
Co-authored-by: Sylvestre Ledru <[email protected]>
09922b8 to
d33be0a
Compare
I am not sure that adding a few strings will regress the performances this way |
|
GNU testsuite comparison: |
I just ran an experiment locally to prove it and got the exact same results. It appears a bigger amount than it actually is on the wall time. Will make another PR to separate out some of the strings from the shared locale bundle to show how the performance improves. You can also see it on the flamecharts for all of the regressions except for sort, that one shows something different somehow. |
|
GNU testsuite comparison: |
|
Just moved it from the uucore locales to the ls locale and that was the only change and all of the regressions vanished |
I have come up with a prototype: #9866 for how to run the GNU SMACK test runners and have a draft of it showing that it can run the tests and pass the examples. To break up this process of enabling SMACK support there are three steps:
First is that even if I have a CI with a SMACK enabled kernel, the smack enabled check uses the ls utility to tell whether the environment has SMACK enabled. This means that even to have the tests changes from SKIP to FAIL I need to have created the fixes for the ls utility.
After this PR is in I am hoping to create a review for the actual CI shell and yml changes to run these tools and then it will change the status from SKIP to FAIL.
Lastly I will need to make the final PR that adds the support for the rest of the utilities and the CI will show it move from FAIL to PASS