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

Build Debian package using debhelper #1200

Merged
merged 18 commits into from
Nov 18, 2022
Merged

Build Debian package using debhelper #1200

merged 18 commits into from
Nov 18, 2022

Conversation

mtlynch
Copy link
Contributor

@mtlynch mtlynch commented Nov 16, 2022

Currently, this doesn't buy us much, but it gets us on the path where we can use dh_installsystemd, which will make it easier to install systemd services instead of doing it manually within maintainer scripts (postinst, presinst).

One of the confusing behaviors of debhelper is how it sees the filesystem root. Without debhelper, all the files outside of the debian meta-directory in the folder /releases/${PKG_ID} would end up in the filesystem of the package (e.g., a file at path /releases/${PKG_ID}/opt/tinypilot/hello.txt would tell the Debian package to install that file to /opt/tinypilot/hello.txt at install time).

With debhelper, the files need to be in /releases/${PKG_ID}/debian/tinypilot at dh_install time, but if we try placing them there when we call dpkg-buildpackage, debhelper can see every path except the folder called tinypilot. As a workaround, we place the package files under /releases/${PKG_ID}/fileroot and then rename the path to /releases/${PKG_ID}/debian/tinypilot just before calling dh_install.

debhelper also strictly requires a changelog file. We can't do a real changelog because we're creating a new Debian package for every commit to the main branch, and we don't want to have to manually upload the changelog on every change, so we just create a dummy changelog at build time.

Note that with this change, the changelog file contains the page version rather than the control file, as it had been previously.

I've diffed the package contents with this change against the master build and verified that the only delta is the addition of the file ./usr/share/doc/tinypilot/changelog.gz.

Review on CodeApprove

Currently, this doesn't buy us much, but it gets us on the path where we can use dh_installsystemd, which will make it easier to install systemd services instead of rolling our own method outside of the Debian package.
@mtlynch mtlynch marked this pull request as ready for review November 17, 2022 10:58
Copy link
Contributor Author

mtlynch commented Nov 17, 2022

Automated comment from CodeApprove ➜

@jdeanwallace please review this Pull Request

Copy link
Contributor

@jdeanwallace jdeanwallace left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

In: debian-pkg/debian/rules:

> Line 14
	dh_install

I managed to build the Debian package without this hack. I've created a POC PR here: #1203

The missing information was that the source files need to be in the build directory (i.e., /releases/${PKG_ID}) and the files we want in the filesystem of the package should be listed in debian/tinypilot.install. Here's a vague explanation.

I also diffed your package contents to mine and they are the same.


In: debian-pkg/Dockerfile:

> Line 60
Architecture: all

Can we use ${PKG_ARCH} here?


In: debian-pkg/Dockerfile:

> Line 63
Description: "Simple, easy-to-use KVM over IP"

The description value is surrounded by quotes. Is that intentional?


👀 @mtlynch it's your turn please take a look

jdeanwallace and others added 5 commits November 17, 2022 15:32
* Build Debian package.

* Use debhelper install file.

* Fix dir copy.

* Fix file copy.

* Move source to base dir.

* Fix debian copy.

* Only copy specific files.

* Try different copyright notice.

* Try different copyright notice.

* Ignore lintian copyright check.
Copy link
Contributor Author

@mtlynch mtlynch left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

In: debian-pkg/debian/compat:

> Line 1
11

Small note - I realized that this refers to the version of debhelper not the version of Debian (like I previously though), so I changed it to 11 since the package declares a Build-Depends on debhelper >= 11.


In: debian-pkg/debian/rules:
Oh, nice! Thanks. That's way cleaner. I've merged your PR into mine.


In: debian-pkg/Dockerfile:
Oh right we just fixed that and I'm messing it up again.


In: debian-pkg/Dockerfile:
Oh, no, fixed.


👀 @jdeanwallace it's your turn please take a look

Copy link
Contributor

@jdeanwallace jdeanwallace left a comment

Choose a reason for hiding this comment

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

Automated comment from CodeApprove ➜

Approved on CodeApprove
✔️ Approved

Thanks for figuring out how to use debhelper 🙌


👀 @mtlynch it's your turn please take a look

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

Successfully merging this pull request may close these issues.

2 participants