Skip to content

build: rename and gate systemd_logind configuration - #13832

Merged
sylvestre merged 2 commits into
uutils:mainfrom
xtqqczze:feat_systemd_logind
Oct 17, 2026
Merged

sylvestre merged 2 commits into
uutils:mainfrom
xtqqczze:feat_systemd_logind

Conversation

@xtqqczze

@xtqqczze xtqqczze commented Aug 9, 2026 •

Copy link
Copy Markdown
Collaborator

Rename the feat_systemd_logind feature to systemd_logind.

Prevent systemd_logind code from being enabled on unsupported platforms, improving cross-platform compilation and bringing the workspace closer to supporting:

cargo check -q --workspace --all-targets --all-features --target x86_64-pc-windows-gnu

@oech3

oech3 commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

We remove it at #11579 .

@github-actions

github-actions Bot commented Aug 9, 2026 •

Copy link
Copy Markdown

Binary size comparison:

Individual binary size comparison VS main (threshold: >=5% AND >=4 KB).

Total size of compared binaries: 148.57 MB (+972 KB, +0.64%)

Significant per-binary changes:
  ls         2.11 MB ->    3.25 MB  (+1.14 MB, +54.36%)
  mkfifo     1.16 MB ->    1.08 MB  (-76 KB, -6.42%)
  mknod      1.16 MB ->    1.09 MB  (-76 KB, -6.38%)
  mkdir      1.16 MB ->    1.09 MB  (-72 KB, -6.04%)

@github-actions

github-actions Bot commented Aug 9, 2026 •

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/date/resolution (fails in this run but passes in the 'main' branch)

@codspeed

codspeed Bot commented Aug 9, 2026 •

Copy link
Copy Markdown

Merging this PR will improve performance by 44.44%

⚠️ 120 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 flame graph

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

⚡ 2 improved benchmarks
✅ 399 untouched benchmarks
⏩ 70 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
⚡ Simulation factor_three_39_bit_primes 593.6 ms 348.4 ms +70.41%
⚡ Simulation factor_five_38_bit_primes 1.9 s 1.5 s +22.44%

Tip

Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.


Comparing xtqqczze:feat_systemd_logind (d86605d) with main (2e28cc3)

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

@xtqqczze
xtqqczze marked this pull request as draft August 9, 2026 19:25
@xtqqczze
xtqqczze force-pushed the feat_systemd_logind branch from ce647a1 to 3cf5970 Compare August 9, 2026 19:29
@xtqqczze
xtqqczze marked this pull request as ready for review August 29, 2026 10:45
@xtqqczze
xtqqczze force-pushed the feat_systemd_logind branch from 3cf5970 to 10225c8 Compare August 29, 2026 11:18
@xtqqczze

This comment was marked as outdated.

@xtqqczze
xtqqczze force-pushed the feat_systemd_logind branch from 10225c8 to 658d6d9 Compare August 29, 2026 16:55
@xtqqczze
xtqqczze force-pushed the feat_systemd_logind branch 2 times, most recently from b1c3a47 to e68a24b Compare September 15, 2026 11:45
@xtqqczze xtqqczze changed the title uucore: add target_os to systemd_logind cfg check build: rename and gate systemd_logind configuration Sep 15, 2026
@oech3

oech3 commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

I think systemd is better if it can be renamed. But I prefer #11579 .

@xtqqczze

Copy link
Copy Markdown
Collaborator Author

@sylvestre Regardless of whether #11579 is desirable (and I think it probably is), I still think we should merge this in the meantime. The merge conflicts should be straightforward to resolve.

@xtqqczze

Copy link
Copy Markdown
Collaborator Author

@sylvestre Could you merge this please; I have changes depending on this.

Comment thread src/uucore/Cargo.toml
@xtqqczze
xtqqczze requested a review from sylvestre September 26, 2026 10:56
@xtqqczze
xtqqczze force-pushed the feat_systemd_logind branch 2 times, most recently from 0bf8d45 to feb5727 Compare September 27, 2026 14:16
@xtqqczze

Copy link
Copy Markdown
Collaborator Author

@sylvestre ready

@xtqqczze
xtqqczze force-pushed the feat_systemd_logind branch from feb5727 to f986fd3 Compare October 7, 2026 18:57
@oech3

oech3 commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

I like --features systemd for the case someone started using libsystemd.so much more (and shorter name).

@xtqqczze

xtqqczze commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

I like --features systemd for the case someone started using libsystemd.so much more (and shorter name).

Currently we are not using general libsystemd functionality; just the systemd-logind subsystem. So it is shorter, but potentially misleading because it suggests broader integration than actually exists.

@oech3

oech3 commented Oct 7, 2026 via email

Copy link
Copy Markdown
Contributor

@xtqqczze

xtqqczze commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

It is not matter since we use openssl instead of openssl_api_name for cksum.

I think we have a feature named openssl to match the crate name, rather than anything particularly well considered.

@xtqqczze
xtqqczze force-pushed the feat_systemd_logind branch from f986fd3 to d86605d Compare October 16, 2026 17:04
@sylvestre
sylvestre merged commit b1f05f1 into uutils:main Oct 17, 2026
234 of 235 checks passed
@sylvestre

Copy link
Copy Markdown
Contributor

Thanks for your PR

@oech3

oech3 commented Oct 17, 2026

Copy link
Copy Markdown
Contributor
> cargo check --features systemd_logind
error: the package 'coreutils' does not contain this feature: systemd_logind
help: packages with the missing feature: uucore, uu_pinky, uu_uptime, uu_users, uu_who

@xtqqczze

Copy link
Copy Markdown
Collaborator Author

error: the package 'coreutils' does not contain this feature: systemd_logind

That's because it's still named feat_systemd_logind for coreutils, since we expect a feat_ prefix for the build script. feat_selinux is the same.

@xtqqczze
xtqqczze deleted the feat_systemd_logind branch October 17, 2026 12:29
@oech3

oech3 commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

Different feature name breaks GNUmakefile when MULTICALL=* was changed.

@xtqqczze

Copy link
Copy Markdown
Collaborator Author

@oech3 Is there any breakage from this PR?

@oech3

oech3 commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

I opened #15454 .

make MULTICALL=y needs CARGOFLAGS=--features=feat_systemd_logind while make MULTICALL=n needs CARGOFLAGS=--features=systemd_logind.

@xtqqczze

Copy link
Copy Markdown
Collaborator Author

@oech3 Sorry, I don't understand the make workflow. Could you please share the command and error message for any breakage.

@oech3

oech3 commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

This is needed for consitency. If downstream wrote the buildscript using make, consumer needs to change feature name too just for changing multicall or individual.

@xtqqczze

Copy link
Copy Markdown
Collaborator Author

If there is no breakage from this PR then let’s discuss in #15454 instead.

@oech3

oech3 commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

Well... changing feature name is always breaking change for build script.

@xtqqczze

Copy link
Copy Markdown
Collaborator Author

I don’t see feat_systemd_logind referenced in any build scripts. Could you point to a specific build script or downstream use that breaks as a result of this change?

@oech3

oech3 commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

make MULTICALL=n UTILS=who CARGOFLAGS=--features=feat_systemd_logind is broken now.
UTILS=who is just an example.

@xtqqczze

Copy link
Copy Markdown
Collaborator Author

I’ll accept that renaming a feature can be a breaking change. However, without a compatibility requirement to preserve the old name, I don’t see why this is a problem. I won’t discuss this further in this thread.

@oech3

oech3 commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

I don't require compatibility for feature name i.e. I'm fine with make MULTICALL=n UTILS=who CARGOFLAGS=--features=systemd_logind. However ,if we use it, we should accept make MULTICALL=y UTILS=who CARGOFLAGS=--features=systemd_logind too.

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.

3 participants