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 python builds #1

Draft
wants to merge 9 commits into
base: develop
Choose a base branch
from
Draft

Conversation

kesslerm
Copy link
Contributor

@kesslerm kesslerm commented Aug 29, 2023

This is a collection of patches to allow the turris python packages to build with OpenWRT main / 23.05. Build failures were mostly due to the new python build process in OpenWRT, which chokes on differences between the OpenWRT and python package names. Other failures were due to the use of pytest-runner, which is now deprecated [1] and not present as a host build dependency.

In most cases, simply forcing the use of the old build system allows packages to be built.

These packages will not build with the new pyproject.toml-based build process
because of their build dependency on pytest-runner.

pytest-runner has been deprecated[1], but instead of adding a package for
pytest-runner to fulfill the build dependency, force these packages to use
the old build process.

[1]: https://github.com/pytest-dev/pytest-runner#deprecation-notice

Signed-off-by: Magnus Kessler <[email protected]>
This package will not build with the pyproject.toml-based build process
because it has a build dependency on pytest-runner[1].

pytest-runner has been deprecated[2], so instead of adding a package for
pytest-runner to fulfill the build dependency, force this package to use
the old build process.

[1]: https://gitlab.nic.cz/turris/python-ubus/-/blob/v0.1.1/setup.py#L35
[2]: https://github.com/pytest-dev/pytest-runner#deprecation-notice

See also: openwrt/packages@4a7173d

Signed-off-by: Magnus Kessler <[email protected]>
Recent versions of the msgpack C-library are installed as msgpack-c.

Signed-of-by: Magnus Kessler <[email protected]>
In OpenWRT main / 23.05 the new build process chokes on
the OpenWRT package names differing from the python package
names. Force the use of the previous build process for now.

Alternative solutions include providing a PYPI_SOURCE_NAME that
contains the python package name, or renaming the OpenWRT packages
to have the same name as the python package.

Signed-off-by: Magnus Kessler <[email protected]>
In OpenWRT main / 23.05 the new build process chokes on
the OpenWRT package names differing from the python package
names. Force the use of the previous build process for now.

Alternative solutions include providing a PYPI_SOURCE_NAME that
contains the python package name, or renaming the OpenWRT packages
to have the same name as the python package.

Also fix up the version number in setup.py, as otherwise
the new build system would generate the wrong whl file and fail.

Signed-off-by: Magnus Kessler <[email protected]>
The previous version 22.3.0 would no longer compile with
OpenWRT 23.0.5/master.

zmq/backend/cython/_version.c:217:12: fatal error: longintrepr.h: No such file or directory
  217 |   #include longintrepr.h
      |            ^~~~~~~~~~~~~~~
compilation terminated.

Signed-off-by: Magnus Kessler <[email protected]>
The new python build system flagged a host build time dependency on Jinja2.
Due to the use of pytest-runner, we use the old build system for now.

In OpenWRT main / 23.05 the new build process chokes on
the OpenWRT package names differing from the python package
names. Force the use of the previous build process for now.

Alternative solutions include providing a PYPI_SOURCE_NAME that
contains the python package name, or renaming the OpenWRT packages
to have the same name as the python package.

Signed-off-by: Magnus Kessler <[email protected]>
In OpenWRT main / 23.05 the new build process chokes on
the OpenWRT package names differing from the python package
names. Force the use of the previous build process for now.

Alternative solutions include providing a PYPI_SOURCE_NAME that
contains the python package name, or renaming the OpenWRT packages
to have the same name as the python package.

Signed-off-by: Magnus Kessler <[email protected]>
Copy link
Contributor

@BKPepe BKPepe left a comment

Choose a reason for hiding this comment

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

Hey @kesslerm,

first I need to say thank you for such contribution, it's almost great as it could be. I do have two hints for you, how it can be even better.

  1. The patches, which fixes compilation (Now it's msgpack-c instead of msgpack or increasing version in setup.py). Should be fixed in upstream repositories.
  • When I was working at CZ.NIC/Turris, then yeah, contributors did not have easy way to contribute to such upstream repositories in the past, but for some time already there are mirrored all repositories in GitHub from their GitLab, so you can fork each relevant repository and submit the PR, there. This avoids patching it downstream, when you need to refresh patches due to offset, fuzzes. It means that Turris needs to release new versions of Sentinel, which can be good for PR. ;)
  1. Nitpick: OpenWRT is wrong spelling, the correct one is OpenWrt.

The rest looks just fine, forcing old build works and that's the best what can be done now.

From cc5b3fe680a35b144e64cf8c1814cd0e0fabb955 Mon Sep 17 00:00:00 2001
From: Magnus Kessler <[email protected]>
Date: Wed, 2 Aug 2023 17:00:25 +0200
Subject: [PATCH] Fix compilation with OpenWRT master / 23.05
Copy link
Contributor

Choose a reason for hiding this comment

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

This patch should be submitted to sentinel-fwlogs repository.

Date: Wed, 2 Aug 2023 17:01:00 +0200
Subject: [PATCH] Fix compilation with OpenWRT master / 23.05

Recent versions of the msgpack C-library are installed as msgpack-c.
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here, this patch should be upstreamed in the relevant repository.

Date: Wed, 2 Aug 2023 14:39:47 +0200
Subject: [PATCH] Fix compilation with OpenWRT master / 23.05

Recent versions of the msgpack C-library are installed as msgpack-c.
Copy link
Contributor

Choose a reason for hiding this comment

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

As well here - patch should be submitted to appropriate repository, in this case, sentinel-proxy.

Date: Wed, 2 Aug 2023 09:08:27 +0200
Subject: [PATCH] python: adjust version number to release version

This is needed so that the correct python wheel archive is generated
Copy link
Contributor

Choose a reason for hiding this comment

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

As well here. It is not advised to increase patches in Turris OS packages repository, when it can be included in upstream repository instead of patching it downstream while refreshing patches due to offset, fuzzes, etc.

@kesslerm kesslerm marked this pull request as draft August 30, 2023 07:24
@kesslerm
Copy link
Contributor Author

Thanks, @BKPepe for the constructive feedback. Indeed, sending patches directly to the upstream repositories appears to be the better way. I can do this, and once those patches are accepted and the repositories are tagged with new releases I will re-spin this PR.

@miska
Copy link
Member

miska commented Aug 30, 2023

Hi,

thank you for your contribution, I merged the following:

8180a96
82612b2
93b6089
7272904
9021fab
22ab058
5c7d2d9

I will merge patches from the following patches to the respective project:

9021fab
5c7d2d9

I will take a closer look at the following commits to make sure it doesn't break 22.03 as we are building hbl from develop as well:

10fd81f
7416047

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

Successfully merging this pull request may close these issues.

3 participants