Skip to content
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

Jump to bottom

systemd: Make build options configurable #53222

Closed
wants to merge 1 commit into from
Closed

systemd: Make build options configurable #53222

wants to merge 1 commit into from

Conversation

jameysharp
Copy link
Contributor

Motivation for this change

The binary closure for systemd is quite large in the default configuration. This patch provides a simple way to turn off build options that aren't needed by some users, removing the corresponding dependencies. It also makes it easy to turn on additional features that NixOS doesn't enable by default.

Since doCheck was disabled, this commit also disables building tests by default. If the caller requests building tests, then doCheck will be turned on as well. The tests are probably still just as broken as they were before, but if someone wants to try them, they can build with features.tests=true.

Things done

The build log from this version is identical to the build log from the previous version, with the following exceptions:

  • Store paths vary, of course.
  • Configure messages like "Dependency libselinux found: NO" are gone; the corresponding checks are explicitly disabled now.
  • Tests aren't built since we don't run them.
  • The build phase compiles files in different orders due to parallel build.
  • The install and fixup phases touch files in different orders, but they touch all the same files.

The man output's closure size is 8 bytes bigger (a less gzip'able store path in a man page somewhere, I guess?) but the other outputs have identical closure sizes.

Also, the same set of paths appear in each output before and after this patch. Their contents differ due to varying store paths so I haven't checked if there are more subtle differences, but based on the above results I suspect they're equivalent.

  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Assured whether relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@jameysharp
systemd: Make build options configurable
fcd9083 
The binary closure for systemd is quite large in the default
configuration. This patch provides a simple way to turn off build
options that aren't needed by some users, removing the corresponding
dependencies. It also makes it easy to turn on additional features that
NixOS doesn't enable by default.

Since doCheck was disabled, this commit also disables building tests by
default. If the caller requests building tests, then doCheck will be
turned on as well. The tests are probably still just as broken as they
were before, but if someone wants to try them, they can build with
features.tests=true.

The build log from this version is identical to the build log from the
previous version, with the following exceptions:

- Store paths vary, of course.
- Configure messages like "Dependency libselinux found: NO" are gone;
  the corresponding checks are explicitly disabled now.
- Tests aren't built since we don't run them.
- The build phase compiles files in different orders due to parallel
  build.
- The install and fixup phases touch files in different orders, but they
  touch all the same files.

Also, the same set of paths appear in each output before and after this
patch. Their contents differ due to varying store paths so I haven't
checked if there are more subtle differences, but based on the above
results I suspect they're equivalent.
Mic92
Mic92 reviewed Jan 2, 2019
View reviewed changes
pkgs/os-specific/linux/systemd/default.nix
"-Dpkgconfigdatadir=$dev/share/pkgconfig"
"-Dpkgconfiglibdir=$dev/lib/pkgconfig"
"-Drootlibdir=$lib/lib"
"-Drootprefix=$out"
Copy link
Member

Choose a reason for hiding this comment

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

using placeholder of nix2 this could be moved to mesonFlags in order to avoid escaping.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for reviewing! I don't know what that means though. Can you give an example or point me at the right documentation?

Copy link
Contributor

Choose a reason for hiding this comment

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

Derivation attributes can now reference the outputs of the derivation using the placeholder builtin function. For example, the attribute

configureFlags = "--prefix=${placeholder "out"} --includedir=${placeholder "dev"}";

will cause the configureFlags environment variable to contain the actual store paths corresponding to the out and dev outputs.

@aneeshusa aneeshusa mentioned this pull request Jan 9, 2019
Closed
7 tasks
@aanderse
Copy link
Member

aanderse commented Aug 4, 2019

@jameysharp any motivation to continue with this PR? From other PRs of yours I'm assuming this PR still has significant value to you.

@jameysharp
Copy link
Contributor Author

Yes! I've been meaning to get back to it. I think the comments about using placeholder got resolved on master already so I just need to rebase and re-test now that master has a new version of systemd.

@doronbehar
Copy link
Contributor

I like this PR as well and wish it would get merged.

@doronbehar
Copy link
Contributor

Also, I'd vote for adding another build flavor for systemd in all-packages.nix perhaps to be named systemdLite and/or systemdFull with less / more features enabled / disabled.

@flokli
Copy link
Contributor

flokli commented Dec 25, 2020

Sorry, seems I missed this PR. Has this been solved by #98998 (and the subsequent PRs)?

@stale
Copy link

stale bot commented Jun 26, 2021

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 26, 2021
@flokli
Copy link
Contributor

flokli commented Jun 26, 2021

Let's close this. It has probably been addressed in other PRs in the meantime, and there was no reaction on #53222 (comment).

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 26, 2021
@flokli flokli closed this Jun 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mic92 Mic92 Mic92 left review comments

@doronbehar doronbehar doronbehar left review comments

@furrycatherder furrycatherder furrycatherder left review comments

Assignees
No one assigned
Labels
2.status: merge conflict
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@jameysharp @aanderse @doronbehar @flokli @Mic92 @furrycatherder @ryantm