refactor: replace rustix::process dependency with rlimit by xtqqczze · Pull Request #14739 · uutils/coreutils · GitHub
Skip to content

refactor: replace rustix::process dependency with rlimit - #14739

Draft
xtqqczze wants to merge 2 commits into
uutils:mainfrom
xtqqczze:GH14664
Draft

refactor: replace rustix::process dependency with rlimit#14739
xtqqczze wants to merge 2 commits into
uutils:mainfrom
xtqqczze:GH14664

Conversation

@xtqqczze

@xtqqczze xtqqczze commented Sep 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Closes #14914.
Reverts #14664.

@sylvestre

sylvestre commented Sep 19, 2026

Copy link
Copy Markdown
Contributor

@xtqqczze xtqqczze mentioned this pull request Sep 19, 2026
Merged
@oech3

oech3 commented Sep 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

I think we don't need to revert this since we were not running test(impossible?) for Redox on the CI previously.

@sylvestre sylvestre closed this Sep 20, 2026
@xtqqczze

xtqqczze commented Sep 20, 2026

Copy link
Copy Markdown
Collaborator Author

I think we should add a check-only job at the very least:

cargo check --all-targets --features feat_os_unix_redox --target x86_64-unknown-redox

We can set continue-on-error: true for now so that any existing errors don’t block CI, while still giving us visibility into Redox build issues.

@sylvestre

sylvestre commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Sounds good

@xtqqczze xtqqczze mentioned this pull request Sep 23, 2026
Open
@xtqqczze

This comment was marked as outdated.

Sign in to view
@oech3

oech3 commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

Do we really need to hurry supporting test on redox? Most logic are shared with other platforms.

@xtqqczze xtqqczze reopened this Sep 28, 2026
@xtqqczze
xtqqczze marked this pull request as draft September 28, 2026 15:21
@xtqqczze
xtqqczze force-pushed the GH14664 branch 2 times, most recently from d2139df to e8df3ba Compare September 28, 2026 16:19
@xtqqczze xtqqczze self-assigned this Sep 28, 2026
@codspeed

codspeed Bot commented Sep 28, 2026 •
edited
Loading

Copy link
Copy Markdown

Merging this PR will regress 1 benchmark

⚠️ 122 benchmarks spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

ℹ️ 1 benchmark has no flamegraph

The flamegraph could not be generated because too much time was spent in runtime internals, making the visualization not useful for performance analysis.

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
❌ 1 regressed benchmark
✅ 399 untouched benchmarks
⏩ 70 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ Memory du_deep_tree[(100, 3)] 76.5 KB 82.4 KB -7.14%
⚡ CPU Simulation true_consecutive_calls 350.2 ns 294.7 ns +18.85%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

Comparing xtqqczze:GH14664 (f8a0e4b) with main (ae34722)2

Open in CodSpeed

Footnotes

  1. 70 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. ↩

  2. No successful run was found on main (198c615) during the generation of this report, so ae34722 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

@xtqqczze xtqqczze mentioned this pull request Sep 28, 2026
Closed
@xtqqczze xtqqczze closed this Sep 28, 2026
@xtqqczze xtqqczze reopened this Oct 11, 2026
@xtqqczze xtqqczze changed the title Revert "tests: replace rlimit dependency with rustix::process" refactor: replace rustix::process dependency with rlimit Oct 11, 2026
@xtqqczze
xtqqczze force-pushed the GH14664 branch 3 times, most recently from 5787bfb to 5489611 Compare October 11, 2026 16:48
@oech3

oech3 commented Oct 11, 2026

Copy link
Copy Markdown
Contributor

If we replaced rustix with rlimit, we need large diff again to remove it after rustix supported rlimit on redox.

@xtqqczze
xtqqczze force-pushed the GH14664 branch 6 times, most recently from 546552d to d71ebe6 Compare October 13, 2026 12:56
@xtqqczze

xtqqczze commented Oct 13, 2026

Copy link
Copy Markdown
Collaborator Author

Split the uutests changes to #14252.

@xtqqczze xtqqczze closed this Oct 13, 2026
@xtqqczze

xtqqczze commented Oct 14, 2026

Copy link
Copy Markdown
Collaborator Author

If we replaced rustix with rlimit, we need large diff again to remove it after rustix supported rlimit on redox.

rustix doesn't seem to be actively maintained. Perhaps we should consider forking it.

@oech3

oech3 commented Oct 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Is there any collaborator for rustix repo in ? We should consider asking maintainer to add collaborator from bytecodealliance.

@oech3

oech3 commented Oct 14, 2026

Copy link
Copy Markdown
Contributor

@xtqqczze xtqqczze mentioned this pull request Oct 14, 2026
Draft
@xtqqczze xtqqczze reopened this Oct 14, 2026
@xtqqczze
xtqqczze force-pushed the GH14664 branch 6 times, most recently from 407f7d4 to d861d13 Compare October 20, 2026 18:25

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@xtqqczze xtqqczze

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

rustix does not support rlimit on Redox

3 participants

@xtqqczze @sylvestre @oech3