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

Replace stdeb in favor of modern Debian packaging tools #901

Merged
merged 3 commits into from
Sep 11, 2024
Merged

Conversation

almet
Copy link
Contributor

@almet almet commented Aug 28, 2024

This removes stdeb in favor of more modern Debian packaging tools (see #773 for the rationale).

As a result, we can build .deb packages on debian trixie.

I've tested manually that the produced debian image is working for the following distributions :

  • ubuntu-22.04
  • ubuntu-23.10
  • ubuntu-24.04
  • debian-bullseye
  • debian-bookworm
  • debian-trixie

ubuntu-20.04 is currently failing because of the dependency to podman that it can't install. I'm not sure yet why.

Fixes #773
Fixes #394
Fixes #621

debian/changelog Outdated Show resolved Hide resolved
debian/compat Show resolved Hide resolved
@almet almet changed the title Add the debian folder generated by stdeb Replace stdeb in favor of modern Debian packaging tools Aug 28, 2024
Copy link
Member

@legoktm legoktm left a comment

Choose a reason for hiding this comment

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

Nice!!! Few notes inline.

You don't need an empty debian/install file, if there's nothing else to add into the package, just delete it.

debian/control Outdated Show resolved Hide resolved
debian/rules Outdated Show resolved Hide resolved
debian/changelog Outdated Show resolved Hide resolved
@almet
Copy link
Contributor Author

almet commented Aug 28, 2024

Thanks for the comments. I've added a few fixup commits addressing them.

@almet
Copy link
Contributor Author

almet commented Aug 28, 2024

ubuntu-20.04 is currently failing because of the dependency to podman that it can't install. I'm not sure yet why.

This seems in fact expected, and we have a script to install podman on focal

@almet
Copy link
Contributor Author

almet commented Sep 2, 2024

I've been curious if the current .deb file we provide to our debian / ubuntu users requires podman, and it does:

Depends: <snip> podman <snip>

I guess it "just" needs to be installed for this to work properly (?)

@almet
Copy link
Contributor Author

almet commented Sep 2, 2024

Actually, the reason for the failure was that we were receiving a 503 error when installing podman from opensuse repositories, so completely orthogonal to our changes here.

debian/control Outdated Show resolved Hide resolved
debian/control Outdated Show resolved Hide resolved
debian/source/options Show resolved Hide resolved
install/linux/build-deb.py Show resolved Hide resolved
@apyrgio
Copy link
Contributor

apyrgio commented Sep 3, 2024

I ran the build-deb.py script, but I encountered the following issues:

First, it seems that pybuild attempts to find and clean .pyc files under the dev_scripts directory. This directory may not be empty if we have created dev environments for Dangerzone:

* Deleting old dist and deb_dist
* Building DEB package
dpkg-buildpackage: info: source package dangerzone
dpkg-buildpackage: info: source version 0.7.0
dpkg-buildpackage: info: source distribution unstable
dpkg-buildpackage: info: source changed by Freedom of the Press Foundation   <info@freedom.press>
dpkg-buildpackage: info: host architecture amd64
 dpkg-source --before-build .
dpkg-source: info: using options from dangerzone/debian/source/options: --compression=gzip --tar-ignore=dev_scripts --tar-ignore=.* --tar-ignore=__pycache__
 debian/rules clean
dh clean --with python3 --buildsystem=pybuild
   dh_auto_clean -O--buildsystem=pybuild
I: pybuild base:240: python3.11 setup.py clean 
running clean
removing '/home/user/dangerzone/.pybuild/cpython3_3.11_dangerzone/build' (and everything under it)
'build/bdist.linux-x86_64' does not exist -- can't clean it
'build/scripts-3.11' does not exist -- can't clean it
find: ‘./dev_scripts/envs/fedora/40/state/containers/storage/overlay/0b59dbe2e0512e172d5728f3e528ab7f43848ff3db0cc591ae8987df56f4fdd5’: Permission denied
find: ‘./dev_scripts/envs/fedora/40/state/containers/storage/overlay/9b1929a41f2d36a472c060e1f346164188e698ca881e124731c2c391ddeac32c’: Permission denied
find: ‘./dev_scripts/envs/fedora/40/state/containers/storage/overlay/a9443b81340f70b00a74a44b30efa594fcd062ec997f2a8cc447f83c0aee9a59’: Permission denied
[...]
rm: remove write-protected regular file './dev_scripts/envs/ubuntu/20.04/state/containers/storage/overlay/93dd5c8a7b4e6f11dfd12cc72a9e0cd53e1b5e688e40aca47b4263f4de645fb3/diff1/opt/dangerzone/dangerzone/conversion/__pycache__/errors.cpython-312.pyc'? 

Second, if you notice in the above logs, it seems that pybuild attempts to use setup.py (python3.11 setup.py clean), instead of consulting pyproject.toml. I think that's the reason why it checks dev_scripts/ in the first place.

Ideally, we'd want to build Dangerzone straight from pyproject.toml for two reasons:

  1. It's the most modern method of building Python packages, and we already use it when we build our RPM packages.
  2. It will soon replace the current setup.py / distutils build method (see pyproject section in https://manpages.debian.org/unstable/dh-python/pybuild.1.en.html#PLUGINS)

The only drawback is that pybuild-plugin-pyproject is not available in Ubuntu Focal. In that case, we may backtrack until Ubuntu Focal is deprecated. Still, it's good to know in the meantime if Dangerzone can be built straight from pyproject.toml.

@almet
Copy link
Contributor Author

almet commented Sep 4, 2024

Thanks for your comments,

First, it seems that pybuild attempts to find and clean .pyc files under the dev_scripts directory. This directory may not be empty if we have created dev environments for Dangerzone

You are correct, and it's annoying to me as well. It's actually debian helper who's using find to look for files, and doesn't get access to some of them, so it's bailing out. I'm trying to find a solution to not include the dev_scripts folder in the find commands, and also disable cleaning files within it.

Second, if you notice in the above logs, it seems that pybuild attempts to use setup.py (python3.11 setup.py clean), instead of consulting pyproject.toml.

I agree 100% with you that using pyproject.toml rather than setup.py is the path forward. I've been actually involved in the early discussions about distutils2 back in the days. It represents something to get rid of this setup.py file whenever I can.

But... I'm unsure if we should be able to build the debian packages on all the platforms we support. If it's the case (and I expect it is), I believe it's better to wait until ubuntu focal is deprecated, like you mentioned.

Nevertheless, I will give a try to pybuild-plugin-pyproject and report my findings, probably in a separate issue.

@apyrgio
Copy link
Contributor

apyrgio commented Sep 5, 2024

I'm trying to find a solution to not include the dev_scripts folder in the find commands, and also disable cleaning files within it.

Cool, cool. I think that if you enable DH_VERBOSE=1, you may get a bit more logs. I believe that the offending command is a python setup.py clean command, and I think we can pass arguments to that via pybuild. Maybe we can even override the clean function from our own setup.py.

But... I'm unsure if we should be able to build the debian packages on all the platforms we support. If it's the case (and I expect it is), I believe it's better to wait until ubuntu focal is deprecated, like you mentioned.

I agree, ultimately that's the sensible thing to do, provided that we make the setup.py (distutils) method work. If not, then pyproject is back on the menu I guess...

@almet
Copy link
Contributor Author

almet commented Sep 5, 2024

I believe that the offending command is a python setup.py clean command, and I think we can pass arguments to that via pybuild. Maybe we can even override the clean function from our own setup.py.

It's actually some of the deb helper make targets which contain some find commands which are trying to read inside some folders they don't have access to. The only way to tweak seems to be by overriding them using *_override targets in d/rules.

I'm still trying to find where the original ones are 👀

@legoktm
Copy link
Member

legoktm commented Sep 5, 2024

The only drawback is that pybuild-plugin-pyproject is not available in Ubuntu Focal. In that case, we may backtrack until Ubuntu Focal is deprecated.

You can probably try something like Build-Depends: pybuild-plugin-project | python3; see https://www.debian.org/doc/debian-policy/ch-relationships.html#relationships-between-source-and-binary-packages-build-depends-build-depends-indep-build-depends-arch-build-conflicts-build-conflicts-indep-build-conflicts-arch for the nuances on how alternative build dependencies work.


The only way to tweak seems to be by overriding them using *_override targets in d/rules.

See https://manpages.debian.org/bookworm/debhelper/dh_auto_clean.1.en.html#DESCRIPTION, especially:

This is intended to work for about 90% of packages. If it doesn't work, or tries to use the wrong clean target, you're encouraged to skip using dh_auto_clean at all, and just run make clean manually.

@almet
Copy link
Contributor Author

almet commented Sep 6, 2024

You can probably try something like Build-Depends: pybuild-plugin-project | python3;

Following this path would mean we would still need to rely on setup.py for ubuntu focal.

Rather than having to maintain multiple paths for this, it might be easier to continue relying on setup.py for the time being, and move to pyproject.toml + pybuild-plugin-project it as soon as we drop support for ubuntu focal, which will happen after 02 April 2025 (the end of Maintenance & Security Support).

I've opened an issue to follow this: #911


Some notes on the current progress:

  1. I've removed python3-all, as it doesn't seem needed anymore to build the debian packages. It seems that the reason we needed it was due to a stdeb bug.

  2. I've removed podman from the build dependencies. It's actually a requirement to build the image, but not a requirement to build the packages themselves. Also, because we're currently considering providing the images for the users to consume, I'm not sure if it's worth adding the dependency now to remove it later on.

  3. The debian package currently builds on all platforms 🤓

@almet almet force-pushed the 773-remove-stdeb branch 2 times, most recently from 47451c1 to 87296f0 Compare September 10, 2024 15:19
.circleci/config.yml Outdated Show resolved Hide resolved
BUILD.md Outdated Show resolved Hide resolved
@apyrgio
Copy link
Contributor

apyrgio commented Sep 10, 2024

I don't have anything else to comment on. Kunal has some nice comments, but other than that, the end result looks great to me. Feel free to merge once Kunal's comment are addressed!

@almet almet force-pushed the 773-remove-stdeb branch 6 times, most recently from c87ccc3 to dce4ce4 Compare September 11, 2024 13:44
As a result, a new `debian` folder is now living in the repository.
Debian packaging is now done manually rather than using tools that do
the heavy-lifting for us.

The `build-deb.py` script has also been updated to use `dpkg-buildpackage`
Previously, these files where stored inside the repository (under
`dev_scripts/env/`), which could lead to conflicts with some tooling
(black, debian-helper).

(Linux only): as a convenience, here is how to move data to the new
location:

```bash
mkdir -p ~/.local/share/dangerzone-dev
mv dev_scripts/envs/ ~/.local/share/dangerzone-dev/.
```
@almet almet merged commit d7f8096 into main Sep 11, 2024
47 of 51 checks passed
@almet almet deleted the 773-remove-stdeb branch September 11, 2024 15:43
@legoktm
Copy link
Member

legoktm commented Sep 11, 2024

Woooo, congrats!! 🥳 🎆 🎊

almet pushed a commit that referenced this pull request Sep 30, 2024
This commit is just to ensure the `hotfix-0.7.1` branch is passing the CI
tests, the actual problem has been solved via [0].

[0] #901
apyrgio added a commit that referenced this pull request Sep 30, 2024
This commit is just to ensure the `hotfix-0.7.1` branch is passing the CI
tests, the actual problem has been solved via [0].

[0] #901
almet pushed a commit that referenced this pull request Oct 1, 2024
This commit is just to ensure the `hotfix-0.7.1` branch is passing the CI
tests, the actual problem has been solved via [0].

[0] #901
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants