Skip to content
This repository has been archived by the owner on May 19, 2022. It is now read-only.

Add debhelper dh_installsystemd style support for systemd unit installation. #135

Merged
merged 8 commits into from
Aug 3, 2020
Merged

Add debhelper dh_installsystemd style support for systemd unit installation. #135

merged 8 commits into from
Aug 3, 2020

Conversation

ximon18
Copy link
Contributor

@ximon18 ximon18 commented Jul 24, 2020

This PR contains a Rust implementation of the Debian debhelper dh_installsystemd functionality which augments or generates maintainer scripts such that when systemd_units is configured, shell script fragments "for enabling, disabling, starting, stopping and restarting systemd unit files" (quoting man 1 dh_installsystemd) will replace the #DEBHELPER# token in the provided maintainer scripts (or provide the entire script if missing).

The README.md has now been updated on how to use this functionality.

Note: I wasn't aware of the discussion in #106 when I wrote this.

There are no tests included in this PR yet. I have some tests I started writing but they are out of date for the code as it is now. I thought it best to first get some feedback and no doubt the code can be improved with review.

I am using this to improve the .deb archives I have created as part of the NLnet Labs Krill project. In that project I have 5 variants and prior to this PR require almost identical asset blocks in each variant in order to select different systemd unit files by target O/S version, and hand coded the maintainer scripts.

With this PR I fix issues in my maintainer scripts by adding systemd unit management shell script fragments exactly how Debian suggests it should be done and also reduce duplication in my Cargo.toml as the variant is used to select the right unit file and add it as an asset automatically.

By default systemd unit files are looked for in the maintainer scripts directory. This can be overriden by providing a units directory to the systemd_units TOML (inline) table. Here's my current systemd_units configuration:


[package.metadata.deb.systemd-units]
unit-name = "krill"
enable = false
start = true
restart-after-upgrade = false
stop-on-upgrade = true

The enable, start, etc options match the options that can be passed to dh_installsystemd.

Example output when run with -v:

$ cargo-deb --variant ubuntu1804 --deb-version 0.8.0~rc1-1bionic -v
info: Stripped '/home/ximon/Documents/code/krill/krill-master/target/release/krill'
info: Stripped '/home/ximon/Documents/code/krill/krill-master/target/release/krillc'
info: - -> usr/share/doc/krill/copyright
info: /home/ximon/Documents/code/krill/krill-master/debian/krill-ubuntu1804.krill.service -> lib/systemd/system/krill.service
info: /home/ximon/Documents/code/krill/krill-master/target/release/krill -> usr/bin/krill
info: /home/ximon/Documents/code/krill/krill-master/target/release/krillc -> usr/bin/krillc
info: /home/ximon/Documents/code/krill/krill-master/defaults/krill.conf -> usr/share/doc/krill/krill.conf
info: /home/ximon/Documents/code/krill/krill-master/doc/krill.1 -> usr/share/man/man1/krill.1
info: /home/ximon/Documents/code/krill/krill-master/doc/krillc.1 -> usr/share/man/man1/krillc.1
info: Found user supplied maintainer script preinst
info: Found user supplied maintainer script postinst
info: Found user supplied maintainer script postrm
info: Determining augmentations needed for systemd unit krill.service
info: Maintainer script postinst will be augmented with autoscript postinst-systemd-dont-enable
info: Maintainer script postrm will be augmented with autoscript postrm-systemd
info: Maintainer script postinst will be augmented with autoscript postinst-systemd-start
info: Maintainer script prerm will be augmented with autoscript prerm-systemd
info: Maintainer script postrm will be augmented with autoscript postrm-systemd-reload-only
info: Augmenting maintainer script postinst
info: Augmenting maintainer script preinst
info: Generating maintainer script prerm
info: Augmenting maintainer script postrm
info: Archiving target/debian/krill-ubuntu1804/systemd/preinst
info: Archiving target/debian/krill-ubuntu1804/systemd/postinst
info: Archiving target/debian/krill-ubuntu1804/systemd/prerm
info: Archiving target/debian/krill-ubuntu1804/systemd/postrm
info: compressed/original ratio 7558272/29141504 (25%)
/home/ximon/Documents/code/krill/krill-master/target/debian/krill_0.8.0~rc1-1bionic_amd64.deb

Feedback very appreciated!

@ximon18
Copy link
Contributor Author

ximon18 commented Jul 24, 2020

I originally wrote this as a separate debhelper crate but wasn't sure that was warranted. I see in the configure me issue that a separate crate was considered.

@ximon18
Copy link
Contributor Author

ximon18 commented Jul 28, 2020

@kornelski: I imagine you are probably just busy, but if there is something I can do to progress this please let me know.

@ximon18
Copy link
Contributor Author

ximon18 commented Jul 29, 2020

@Kixunil: on #106 you said:

Second, I was thinking about adding debconf support to configure_me, so it can automatically generate templates and postinst script (possibly another binary, though)

This PR might interest you as it implements Debian debhelper (link) dh_installsystemd (link) logic for maintainer script augmentation/generation. I initially wrote it as a separate crate but as the only user I was aware of at the time was cargo deb that seemed overkill, but if there are other crates that want that logic it could be extracted to a crate.

@Kixunil
Copy link

Kixunil commented Jul 29, 2020

Thanks for heads up! I redesigned my projects quite a bit since I wrote it. I want to stay close to Debian policy and Using anything else than dpkg-buildpackage doesn't seem like a safe approach. That being said, I am probably not 100% compliant as that's too much burden to me right now (I hope to improve in the future).

I'm now developing a separate packaging helper with those goals in mind.

@ximon18 ximon18 mentioned this pull request Jul 30, 2020
@ximon18
Copy link
Contributor Author

ximon18 commented Jul 30, 2020

I considered making this new behaviour the default, i.e. that cargo deb would be enough to detect any systemd unit files in a debian subdirectory if present and to generate or augment maintainer scripts in debian and it would all just work. However, though perhaps this is not very likely, if someone had a debian directory containing systemd units that they did NOT want to be installed with the package (perhaps because the directory is shared between multiple packages or projects?) this new functionality would cause the package to be built differently than prior to this PR.

src/dh_lib.rs Outdated Show resolved Hide resolved
src/dh_lib.rs Outdated Show resolved Hide resolved
src/dh_lib.rs Outdated Show resolved Hide resolved
let mut file = if !outfile.exists() {
File::create(&outfile).unwrap()
} else {
OpenOptions::new().append(true).open(&outfile).unwrap()
Copy link
Collaborator

Choose a reason for hiding this comment

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

cargo-deb keeps other generated assets in memory, without needing to use a temp directory.

Writing stuff to and from file system looks cumbersome. Maybe it'd be simpler to have the scripts as String in memory and just append lines to it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I considered that and agree with you. I didn't try that yet as I wanted first to get the debhelper dh_systemdinstall functionality working as it does in the original Perl implementation, then ensure I have tests passing that verify the working behaviour, then change the implementation to be in-memory based. I am happy to try updating it to work in-memory instead of with a temp folder.

autoscripts/README.md Outdated Show resolved Hide resolved
@kornelski kornelski merged commit db4a33c into mmstick:master Aug 3, 2020
@ximon18
Copy link
Contributor Author

ximon18 commented Aug 3, 2020

Thanks!

@ximon18 ximon18 deleted the dh_installsystemd branch August 3, 2020 13:11
@kornelski
Copy link
Collaborator

Thank you.

Could you follow it up with a change to use in-memory objects instead of file rewrites and appends?

@ximon18
Copy link
Contributor Author

ximon18 commented Aug 3, 2020

Yes, I was just working on that :-)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants