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

fix(systemd): add Before=shutdown.target to cloud-init-main.service.tmpl #5653

Merged
merged 1 commit into from
Sep 4, 2024

Conversation

aciba90
Copy link
Contributor

@aciba90 aciba90 commented Sep 2, 2024

Proposed Commit Message

fix(systemd): add Before=shutdown.target to cloud-init-main.service.tmpl

Fixes lintian warning systemd-service-file-shutdown-problems.
See [1, 2].

Remove superflous conditional additions of Conflicts=shutdown.target as
it is unconditionally added for every target.

[1] https://salsa.debian.org/lintian/lintian/-/blob/2.118.0/tags/s/systemd-service-file-shutdown-problems.tag
[2] https://github.com/systemd/systemd/issues/11821

Additional Context

Test Steps

$ DEB_BUILD_OPTIONS=nocheck make deb
$ lintian cloud-init_all.deb 
...
W: cloud-init: systemd-service-file-shutdown-problems [usr/lib/systemd/system/cloud-init-main.service]

Merge type

  • Squash merge using "Proposed Commit Message"
  • Rebase and merge unique commits. Requires commit messages per-commit each referencing the pull request number (#<PR_NUM>)

Fixes lintian warning systemd-service-file-shutdown-problems. See [1,
2].

Remove superflous conditional additions of Conflicts=shutdown.target as
it is unconditionally added for every target.

[1] https://salsa.debian.org/lintian/lintian/-/blob/2.118.0/tags/s/systemd-service-file-shutdown-problems.tag
[2] systemd/systemd#11821
@aciba90 aciba90 requested a review from holmanb September 2, 2024 10:22
Copy link
Member

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

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

I think we want the Before alongside the Conflicts. The DefaultDependencies=no means those Conflicts lines aren't implicitly added.

From some random source I found online:

By default, all units and scopes have DefaultDependencies=yes, which implicitly adds Before=shutdown.target and Conflicts=shutdown.target to them. Conflicts means starting shutdown.target stops conflicting units. Starting shutdown.target will stop all conflicting units in parallel (unless otherwise ordered).

@TheRealFalcon TheRealFalcon self-assigned this Sep 3, 2024
@aciba90
Copy link
Contributor Author

aciba90 commented Sep 4, 2024

@TheRealFalcon, yes. We need both Before=shutdown.target and Conflicts=shutdown.target present with DefaultDependencies=no. And that is already the case, the Conflicts one is in [1]. I am removing the other Conflicts lines because they are noops, as they apply under certain distros, but [1] add it unconditionally, so they are redundant.

[1] https://github.com/canonical/cloud-init/pull/5653/files#diff-93beecb98a5a39f70a7be0a2928beaee4bc92f85befa463b0bc40a340ff2e1c6R29

@aciba90 aciba90 mentioned this pull request Sep 4, 2024
2 tasks
Copy link
Member

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

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

Ah thanks...sorry, I didn't it on the other line at first.

@aciba90 aciba90 merged commit 9cc458c into canonical:main Sep 4, 2024
22 checks passed
@aciba90 aciba90 deleted the oracular-upstream-lintian-fixes branch September 4, 2024 14:11
@blackboxsw blackboxsw added the packaging Supplemental package review requested label Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packaging Supplemental package review requested
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants