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 RPM packages from an RPM SPEC #538

Merged
merged 11 commits into from
Sep 20, 2023
Merged

Build RPM packages from an RPM SPEC #538

merged 11 commits into from
Sep 20, 2023

Conversation

apyrgio
Copy link
Contributor

@apyrgio apyrgio commented Aug 31, 2023

This PR replaces the previous way we packaged Dangerzone for Fedora (via bdist_rpm, see #298) with a SPEC file that uses Python macros.

This way, we can create an RPM package in a fully PEP-517 compliant way, without using setuptools and setup.py. By doing so, we have much more control on the resulting RPM, we fix the issue of the deprecated bdist_rpm command, as well as some RPM issues that derived from that fact.

Fixes #298
Fixes #514

@apyrgio apyrgio force-pushed the 298-rpm-spec branch 3 times, most recently from 2ebc8ca to 5bd5962 Compare August 31, 2023 15:03
@apyrgio apyrgio added this to the 0.5.0 milestone Sep 7, 2023
Copy link
Contributor

@deeplow deeplow left a comment

Choose a reason for hiding this comment

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

Thanks for figuring out the whole spec file thing. I still haven't managed to build it, but I'm sending already some feedback from the visual review.

.gitignore Show resolved Hide resolved
install/linux/dangerzone.spec Outdated Show resolved Hide resolved
install/linux/dangerzone.spec Show resolved Hide resolved
install/linux/build-rpm.py Outdated Show resolved Hide resolved
install/linux/dangerzone.spec Show resolved Hide resolved
@deeplow
Copy link
Contributor

deeplow commented Sep 12, 2023

While trying to debug why I can't build the package on my system, the fact that it creates temporary directories and I have to navigate onto them become a bit annoying. I can't just cd into it. First I need to find the tempdir name and if I've run multiple times, now there are multiple temp dirs.

What do you think about replacing the --keep flag and temporary directory system with just a permanent location which gets removed upon rebuild and is gitignored.

@deeplow
Copy link
Contributor

deeplow commented Sep 12, 2023

As mentioned before, I wasn't able to build this locally in my Fedora 38. So it may be something wrong with my configuration. However, I was able to build it in the build environement (created by env.py), so I think it's fine to go ahead a continue with that issue pending as I have already spent enought time trying to understand why rpm's wheel building fails to include the sources.

@apyrgio
Copy link
Contributor Author

apyrgio commented Sep 14, 2023

First I need to find the tempdir name and if I've run multiple times, now there are multiple temp dirs.

Yeah, I was thinking about this as well. Personally, I get the temporary directory from the command that failed, but it's not that easy to parse.

What do you think about replacing the --keep flag and temporary directory system with just a permanent location which gets removed upon rebuild and is gitignored.

Hm, we can do it. It's just that if you cd into that directory, you then need to cd out and back again, which is weird. If you don't have a problem with that though (I can manage), I can update this PR.

@apyrgio
Copy link
Contributor Author

apyrgio commented Sep 18, 2023

Hm, we can do it. It's just that if you cd into that directory, you then need to cd out and back again, which is weird. If you don't have a problem with that though (I can manage), I can update this PR.

Well, we can just remove the contents of the dirs, which will not affect cd.

In order to let isort respect .gitignore, we need to specify this in the
tool.isort entry, in pyproject.toml.

For black, we don't need any extra tweaks. This is weird, since until a
few months ago black did not respect .gitignore. Maybe something has
changed in the meantime but if not, we should revert this change.
The dangerzone-container entrypoint, as specified in pyproject.toml, is
stale, for the following reasons:

1. It's not mentioned in the setup.py script, so it was never included
   in our Linux distributions.
2. The code in `dangerzone.__init__.py` that decides if it will invoke
   the GUI or CLI backend, just takes `dangerzone-cli` into account for
   this decision, and does not mention dangerzone-container anywhere.
Update the `build-backend` attribute, in accordance with the Python
Poetry docs [1]. Also, bump the minimum required poetry-core version to
1.2.0, since this is the version that introduced the Poetry dependency
groups [2], i.e., the [tool.poetry.group] sections in pyproject.toml.

[1]: https://python-poetry.org/docs/pyproject/#poetry-and-pep-517
[2]: https://python-poetry.org/docs/managing-dependencies/#dependency-groups
Update our pyproject.toml file to include some non-Python data files,
e.g., our container image and assets. This way, we can use `poetry
build` to create a source distribution / Python wheel from our source
repository.

Note that this list of data files is already defined in our `setup.py`
script. In that script, one can find some extra goodies:

1. We can conditionally include data files in our Python package. We use
   this to include Qubes data only in our Qubes packages.
2. We can specify where will the data files be installed in the end-user
   system.

The above are non-goals for Poetry [1], especially (2), because modern
Python wheels are not supposed to install files in arbitrary places
within the user's host, nor should the install invocation use sudo.
Instead, this is a task that's better suited for the .deb / .rpm
packages.

So, why do we bother updating our `pyproject.toml` and not use
`setup.py` instead? Because `setup.py` is deprecated [2,3], and the
latest Python packaging RFCs [4], as well as most recent Fedora
guidelines [5] use `pyproject.toml` as the source of truth, instead of
`setup.py`.

In subsequent commits, we will also use just `pyproject.toml` for RPM
packaging.

[1]: python-poetry/poetry#890
[2]: https://peps.python.org/pep-0517/#source-trees
[3]: https://blog.ganssle.io/articles/2021/10/setup-py-deprecated.html
[4]: https://peps.python.org/pep-0517/
[5]: https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/
Introduce a SPEC file that can be used to create an RPM from a Python
source distribution. Some notable features of this SPEC file follow:

1. We can use this SPEC file to create both regular RPM packages and
   ones targeted for Qubes.
2. It has a post installation script that removes stale .egg-info
   directories, which previously caused issues to our users.
3. It automatically creates a changelog from our Git logs, which differs
   from the actual CHANGELOG.md.
4. It folloes the latest Fedora guidelines (as of writing this) for
   packaging Python projects.

Fixes #514
Update the dependencies required to build RPM packages. More
specifically, remove the older python3-setuptools dependency, and depend
instead on python3-devel and python3-poetry-core.

Note that this commit may break our CI, but it will be resolved in
subsequent commits.
Add an `rpm-build` directory under `install/linux`, which will be used
for building Dangerzone RPMs. For the time being, it only has a
.gitignore file there, but in the future, invoking
`install/linux/build-rpm.py` will populate it.
Replace the deprecated `bdist_rpm` method of creating RPMs for
Dangerzone. Instead, update our `install/linux/build-rpm.py` script, to
build Dangerzone RPMs using our SPEC file under
`install/linux/dangerzone.spec`. The script now essentially creates a
source distribution (sdist) using `poetry build`, and then uses
`rpmbuild` to create binary and source RPMs.

Fixes #298
@apyrgio apyrgio merged commit fbe13bb into main Sep 20, 2023
@apyrgio apyrgio deleted the 298-rpm-spec branch October 2, 2023 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants