-
Notifications
You must be signed in to change notification settings - Fork 18.6k
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
hack: explicitly control enabling the journald logging driver #47789
base: master
Are you sure you want to change the base?
hack: explicitly control enabling the journald logging driver #47789
Conversation
Sorry, I'm actually struggling to understand here. If you want to build a binary that works for users who may or may not be using systemd (and thus may or may not want the journald logging driver), you need to compile against libsystemd and libsystemd is then a dependency of that binary (and binary package). The "fix" here is to accurately make |
... but the fix broke functionality of the binary for the other half 😭 |
No, it didn't, because we wired up the packaging to pass a flag to enable/disable systemd based on a packaging option (and I verified libsystemd gets linked against correctly when it's on, and not when it's off). What we want is a toggle supported upstream to say "yes, I know systemd is installed on this system, please don't use it" to produce a binary which isn't linked against libsystemd, rather than deciding purely based on whether it's installed. The default doesn't matter for us either way, I presume you'd want it on. Our documentation on it is https://wiki.gentoo.org/wiki/Project:Quality_Assurance/Automagic_dependencies. |
@tianon this is a very big misunderstanding of feature flags in general, optional dependencies (and automatic ones) and overall is exactly the opposite of what Gentoo wants. Gentoo has a robust mechanism for linking feature flags on the distro package to compiled-in features in the binary. In order for this to work, the docker build script needs to expose the option. The current state of affairs is that Gentoo's binary caching layer marks some binaries as being linked to systemd and other binaries as being NOT linked to systemd, but the docker build system is ignoring that and building both against systemd. End result: people disable the Gentoo feature flag "systemd" but accidentally get a broken binary that was linked against systemd just because it happened to be installed on a buildbot VM. It works if they totally disable binary caching because then docker gets built from source and end users who don't have the "systemd" feature flag set on their Gentoo install usually don't also have systemd installed. (It's not a guarantee.) The goal of this PR is so that Gentoo can forward the package "systemd" toggle to the docker build script so that both the package manager and the docker binary agree on what feature flags are currently enabled. |
fc6c657
to
221133b
Compare
I updated the patch to have this on by default as @thesamesam suggested above. |
Looks like the commit is missing a DCO sign-off, causing CI to fail. If you update, could you also remove the |
Counterpoint: it also makes it harder to tell from the git log, where the related discussion is. |
Without this, the dependency on systemd is said to be "automagic", which can lead to breakage, for example, if a binary package of docker is built on a system that has systemd installed then installed on a system that does not have systemd installed. for example: https://bugs.gentoo.org/914076 Signed-off-by: William Hubbs <[email protected]>
221133b
to
890be8a
Compare
I added the DCO signoff and removed the Closes keyword as requested. |
Ping, what is the status of this pr? Thanks much. |
Without this, the dependency on systemd is said to be "automagic", which can lead to breakage, for example, if a binary package of docker is built on a system that has systemd installed then installed on a system that does not have systemd installed.
for example: https://bugs.gentoo.org/914076
closes #47770
- What I did
- How I did it
- How to verify it
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)