Skip to content

php: fix systemdLibs dependency#433641

Merged
r-vdp merged 1 commit into
NixOS:stagingfrom
vog:php-fix-systemdlibs-dependency
Aug 18, 2025
Merged

php: fix systemdLibs dependency#433641
r-vdp merged 1 commit into
NixOS:stagingfrom
vog:php-fix-systemdlibs-dependency

Conversation

@vog

@vog vog commented Aug 14, 2025

Copy link
Copy Markdown
Contributor

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

Add a 👍 reaction to pull requests you find important.

@nixpkgs-ci nixpkgs-ci Bot added 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 501-1000 This PR causes many rebuilds on Linux and should normally target the staging branches. 6.topic: php PHP is a general-purpose scripting language geared towards web development. labels Aug 14, 2025
@nix-owners nix-owners Bot requested a review from piotrkwiecinski August 14, 2025 12:05
@drupol

drupol commented Aug 14, 2025

Copy link
Copy Markdown
Contributor

Care to explain what this is fixing ?

@aanderse aanderse left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@drupol - php doesn't use too much from systemd, just the sd-daemon stuff so we can just pull in the lighter systemdLibs instead of the full systemd closure


fun fact: there are even independent implementations of the basic sd-daemon stuff which can be substituted in place of systemd entirely

@drupol

drupol commented Aug 14, 2025

Copy link
Copy Markdown
Contributor

It would be nice, just for my own info, to compare the closure size, before and after this patch.

@nixpkgs-ci nixpkgs-ci Bot added 12.approvals: 2 This PR was reviewed and approved by two persons. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. labels Aug 14, 2025
@aanderse

Copy link
Copy Markdown
Member

It would be nice, just for my own info, to compare the closure size, before and after this patch.

yes! it would be great to see some real numbers on that! to be clear though... the improvements only help users who aren't running nixos... and only if no other dependencies are pulling in systemd, of course

please do share your results if you don't mind @drupol ❤️

i miss the days when the PR template asked users to include that information... 😅

@drupol

drupol commented Aug 14, 2025

Copy link
Copy Markdown
Contributor

I have no access to a computer these days, just a smartphone... So I could do it, but it need to wait until the 25.

@aanderse

Copy link
Copy Markdown
Member

before patch:

aaron@framework ~/nixpkgs (master)> nix path-info -Sh .#php
/nix/store/s4kv9bzqawhqcyjg9xm2hji3jap6d6kd-php-with-extensions-8.4.11   357.0 MiB

after patch:

aaron@framework ~/nixpkgs (master)> nix path-info -Sh .#php
/nix/store/dh375svy6ycjlir3xqg7nwvrzkq9s7v3-php-with-extensions-8.4.11   251.8 MiB

@vog our friends running php outside of nixos thank you for your service 🫡

@drupol

drupol commented Aug 14, 2025

Copy link
Copy Markdown
Contributor

Amazing improvements, thanks !

@r-vdp

r-vdp commented Aug 15, 2025

Copy link
Copy Markdown
Contributor

Could you retarget this PR on staging please (PR causes a lot of rebuilds and doesn't seem urgent)? See CONTRIBUTING.md in the root of this repo for how to do that if this is your first time.

@vog vog force-pushed the php-fix-systemdlibs-dependency branch from 4e5b5d4 to 0456b29 Compare August 16, 2025 13:21
@vog vog changed the base branch from master to staging August 16, 2025 13:21
@nixpkgs-ci nixpkgs-ci Bot closed this Aug 16, 2025
@nixpkgs-ci nixpkgs-ci Bot reopened this Aug 16, 2025
@vog

vog commented Aug 16, 2025

Copy link
Copy Markdown
Contributor Author

Could you retarget this PR on staging please (PR causes a lot of rebuilds and doesn't seem urgent)? See CONTRIBUTING.md in the root of this repo for how to do that if this is your first time.

@r-vdp Done.

@r-vdp

r-vdp commented Aug 18, 2025

Copy link
Copy Markdown
Contributor

Merging despite CI failures due to what was explained by @wolfgangwalther in #433749

@r-vdp r-vdp merged commit f444f4c into NixOS:staging Aug 18, 2025
63 of 85 checks passed
@nixpkgs-ci

nixpkgs-ci Bot commented Aug 18, 2025

Copy link
Copy Markdown
Contributor

Successfully created backport PR for staging-25.05:

@github-actions github-actions Bot added the 8.has: port to stable This PR already has a backport to the stable release. label Aug 18, 2025
@drupol

drupol commented Sep 1, 2025

Copy link
Copy Markdown
Contributor

@vog I tried to replace systemd with systemdLibs in util-linux and it looks like everything works.

The gain in size is significant:

❯ nix path-info -Sh ./result-bin
/nix/store/qx5wjl2d6bpy82vwkvv9hfm3nfbsm44r-util-linux-2.41.1-bin         88.1 MiB
❯ jj new master
❯ nix-build -A util-linux
/nix/store/ad5skbqn01fbmgwd0b879zy5x5skgxj1-util-linux-2.41.1-bin
❯ nix path-info -Sh ./result-bin
/nix/store/ad5skbqn01fbmgwd0b879zy5x5skgxj1-util-linux-2.41.1-bin        206.7 MiB

How to make sure doing such a thing won't break anything?

@vog

vog commented Sep 1, 2025

Copy link
Copy Markdown
Contributor Author

@vog I tried to replace systemd with systemdLibs in util-linux and it looks like everything works.

The gain in size is significant:

❯ nix path-info -Sh ./result-bin
/nix/store/qx5wjl2d6bpy82vwkvv9hfm3nfbsm44r-util-linux-2.41.1-bin         88.1 MiB
❯ jj new master
❯ nix-build -A util-linux
/nix/store/ad5skbqn01fbmgwd0b879zy5x5skgxj1-util-linux-2.41.1-bin
❯ nix path-info -Sh ./result-bin
/nix/store/ad5skbqn01fbmgwd0b879zy5x5skgxj1-util-linux-2.41.1-bin        206.7 MiB

How to make sure doing such a thing won't break anything?

This is great!

Given that util-linux independent from systemd, and on many systems compiled completely without systemd support at all, I don't see any risk here if it compiles without any errors.

Also note that this is really mostly affecting people using nixpkgs as package manager, not users of NixOS - because NixOS, as OS, ships with a full systemd anyways.

As a general note, I have the impression that practically all "normal" packages (i.e. not Nix modules or similar) that depend on the "systemd" derivation do so simply because they didn't notice the "systemdLibs" derivation that they are actually supposed to have depended on.

@vog vog deleted the php-fix-systemdlibs-dependency branch September 7, 2025 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: php PHP is a general-purpose scripting language geared towards web development. 8.has: port to stable This PR already has a backport to the stable release. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 501-1000 This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 12.approvals: 2 This PR was reviewed and approved by two persons. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants