Skip to content

Modern Python packaging #147

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.

Sign up for GitHub

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

Closed
wants to merge 6 commits into from
Closed

Modern Python packaging #147

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
909e16c
Modern Python packaging
hongquan Jun 23, 2025
58bad8c
Reformat Meson file
hongquan Jun 25, 2025
c6e3143
Convert tasks from Makefile to Meson
hongquan Jun 25, 2025
af80340
Move tasks from Makefile to Meson
hongquan Jun 25, 2025
7d34abe
Update README
hongquan Jun 26, 2025
231abcd
Trick to keep Meson from demanding other tools when building Python e…
hongquan Jun 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 22 additions & 16 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,39 +2,48 @@
# vi: ts=2 sw=2 et:
# SPDX-License-Identifier: LGPL-2.1-or-later
#
name: Build test
name: Build
on:
pull_request:
push:
branches:
- main
- feature/meson-build
Copy link
Member

@keszybz keszybz Jun 28, 2025

Choose a reason for hiding this comment

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

??

Copy link
Contributor Author

@hongquan hongquan Jun 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This is just to make GH Action run on my fork repo when I push the code.
The line will be deleted when the PR is approved.


permissions:
contents: read

jobs:
build:
runs-on: ubuntu-20.04
concurrency:
group: ${{ github.workflow }}-${{ matrix.python }}-${{ github.ref }}
cancel-in-progress: true
strategy:
fail-fast: false
matrix:
os: ["ubuntu-22.04", "ubuntu-24.04"]
python: [
"3.7",
"3.8",
"3.9",
"3.10",
"3.11.0-rc.1",
"3.11",
"3.12",
"3.13",
]
name: Python ${{ matrix.python }}
exclude:
- os: "ubuntu-22.04"
python: "3.13"
- os: "ubuntu-24.04"
python: "3.7"
Copy link
Member

@keszybz keszybz Jun 28, 2025

Choose a reason for hiding this comment

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

3.7 is very old at this point. I guess we could drop that. But OTOH, the code will work fine with those old versions, so it should be fine to keep testing and supporting those old versions until there's some particular reason to raise the minimum.

runs-on: ${{ matrix.os }}
concurrency:
group: ${{ github.workflow }}-${{ matrix.python }}-${{ matrix.os }}-${{ github.ref }}
cancel-in-progress: true
name: Python ${{ matrix.python }} on ${{ matrix.os }}
steps:
- name: Repository checkout
uses: actions/checkout@v2
uses: actions/checkout@v4

- name: Configure Python ${{ matrix.python }}
uses: actions/setup-python@v2
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python }}
architecture: x64
Expand All @@ -43,14 +52,11 @@ jobs:
run: |
sudo apt -y update
sudo apt -y install gcc libsystemd-dev
python -m pip install pytest sphinx
python -m pip install --break-system-packages pytest sphinx build

- name: Build (Python ${{ matrix.python }})
run: |
set -x
make -j
make doc SPHINXOPTS="-W -v"

- name: Test (Python ${{ matrix.python }})
run: |
make check
python -m build -Cbuild-dir=_build
python -m pip install .
cd _build && python -m sphinx -b html -W -v ../docs html
11 changes: 6 additions & 5 deletions .github/workflows/codeql.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ on:
push:
branches:
- main
- feature/meson-build
Copy link
Member

@keszybz keszybz Jun 28, 2025

Choose a reason for hiding this comment

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

??

pull_request:
branches:
- main
Expand All @@ -18,7 +19,7 @@ permissions:
jobs:
analyze:
name: Analyze
runs-on: ubuntu-22.04
runs-on: ubuntu-24.04
concurrency:
group: ${{ github.workflow }}-${{ matrix.language }}-${{ github.ref }}
cancel-in-progress: true
Expand All @@ -33,10 +34,10 @@ jobs:

steps:
- name: Checkout repository
uses: actions/checkout@v3
uses: actions/checkout@v4

- name: Initialize CodeQL
uses: github/codeql-action/init@v2
uses: github/codeql-action/init@v3
with:
languages: ${{ matrix.language }}
queries: +security-extended,security-and-quality
Expand All @@ -47,7 +48,7 @@ jobs:
sudo apt -y install gcc libsystemd-dev

- name: Autobuild
uses: github/codeql-action/autobuild@v2
uses: github/codeql-action/autobuild@v3

- name: Perform CodeQL Analysis
uses: github/codeql-action/analyze@v2
uses: github/codeql-action/analyze@v3
18 changes: 9 additions & 9 deletions .github/workflows/install.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@
# vi: ts=2 sw=2 et:
# SPDX-License-Identifier: LGPL-2.1-or-later
#
name: Install test
name: Install then test
on:
pull_request:
push:
branches:
- main
- feature/meson-build
Copy link
Member

@keszybz keszybz Jun 28, 2025

Choose a reason for hiding this comment

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

??


permissions:
contents: read
Expand All @@ -24,16 +25,16 @@ jobs:
container: [
"archlinux:latest",
"debian:testing",
"quay.io/centos/centos:stream8",
"quay.io/centos/centos:stream10",
"quay.io/fedora/fedora:rawhide",
"ubuntu:focal",
"ubuntu:noble",
]
container:
image: ${{ matrix.container }}
name: ${{ matrix.container }}
steps:
- name: Repository checkout
uses: actions/checkout@v2
uses: actions/checkout@v4

- name: Install dependencies
shell: bash
Expand All @@ -51,8 +52,7 @@ jobs:

case "$DIST_ID" in
arch)
pacman --noconfirm -Sy "${DEPS_COMMON[@]}" systemd-libs
python3 -m ensurepip
pacman --noconfirm -Sy "${DEPS_COMMON[@]}" systemd-libs python-pip
;;
centos|fedora)
dnf -y install "${DEPS_COMMON[@]}" systemd-devel python3-devel python3-pip
Expand All @@ -66,13 +66,13 @@ jobs:
exit 1
esac

python3 -m pip install pytest sphinx
python3 -m pip install --break-system-packages pytest sphinx

- name: Build & install
- name: Install & test
shell: bash
run: |
set -x
python3 -m pip install -I -v .
python3 -m pip install -I -v --break-system-packages .
# Avoid importing the systemd module from the git repository
cd /
python3 -c 'from systemd import journal; print(journal.__version__)'
Expand Down
4 changes: 4 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ __pycache__/
*.egg
*.egg-info
dist
_build
build
Comment on lines +10 to 11
Copy link
Member

@keszybz keszybz Jun 28, 2025

Choose a reason for hiding this comment

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

I see both build and _build here. We normally use build for the build directory. I'd prefer not to introduce _build.

eggs
parts
Expand All @@ -30,3 +31,6 @@ pip-log.txt

#Mr Developer
.mr.developer.cfg

# Zed
.zed
32 changes: 11 additions & 21 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,7 @@ INCLUDE_DIR := $(shell pkg-config --variable=includedir libsystemd)
INCLUDE_FLAGS := $(shell pkg-config --cflags libsystemd)
VERSION := $(shell $(PYTHON) setup.py --version)
TESTFLAGS = -v

define buildscript
import sys, sysconfig, setuptools
sversion = int(setuptools.__version__.split(".")[0])
end = sys.implementation.cache_tag if sversion >= 61 else "{}.{}".format(*sys.version_info[:2])
print("build/lib.{}-{}".format(sysconfig.get_platform(), end))
endef

builddir := $(shell $(PYTHON) -c '$(buildscript)')
BUILD_DIR = _build
Copy link
Member

@keszybz keszybz Jun 28, 2025

Choose a reason for hiding this comment

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

Please keep build.


all: build

Expand All @@ -32,38 +24,36 @@ update-constants: update-constants.py $(INCLUDE_DIR)/systemd/sd-messages.h
mv docs/id128.rst{.tmp,}

build:
$(PYTHON) setup.py build_ext $(INCLUDE_FLAGS)
$(PYTHON) setup.py build
$(PYTHON) -m build -Cbuild-dir=$(BUILD_DIR)

install:
$(PYTHON) setup.py install --skip-build $(if $(DESTDIR),--root $(DESTDIR))
$(PYTHON) -m pip install .

dist:
$(PYTHON) setup.py sdist
$(PYTHON) -m build --sdist

sign: dist/systemd-python-$(VERSION).tar.gz
gpg --detach-sign -a dist/systemd-python-$(VERSION).tar.gz

clean:
rm -rf build systemd/*.so systemd/*.py[co] *.py[co] systemd/__pycache__
rm -rf _build systemd/*.so systemd/*.py[co] *.py[co] systemd/__pycache__

distclean: clean
rm -rf dist MANIFEST

SPHINXOPTS += -D version=$(VERSION) -D release=$(VERSION)
sphinx-%: build
cd build && \
PYTHONPATH=../$(builddir) $(PYTHON) -m sphinx -b $* $(SPHINXOPTS) ../docs $*
@echo Output has been generated in build/$*
sphinx-%: install
Copy link
Member

@keszybz keszybz Jun 28, 2025

Choose a reason for hiding this comment

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

Hmm, this seems wrong. You now require install to be run before building docs.

cd $(BUILD_DIR) && $(PYTHON) -m sphinx -b $* $(SPHINXOPTS) ../docs $*
@echo Output has been generated in $(BUILD_DIR)/$*

doc: sphinx-html

check: build
(cd $(builddir) && $(PYTHON) -m pytest . ../../docs $(TESTFLAGS))
(cd $(BUILD_DIR) && $(PYTHON) -m pytest ../systemd/test ../docs $(TESTFLAGS))

www_target = www.freedesktop.org:/srv/www.freedesktop.org/www/software/systemd/python-systemd
doc-sync:
rsync -rlv --delete --omit-dir-times build/html/ $(www_target)/
rsync -rlv --delete --omit-dir-times $(BUILD_DIR)/html/ $(www_target)/

upload: dist/systemd-python-$(VERSION).tar.gz dist/systemd-python-$(VERSION).tar.gz.asc
twine-3 upload $+
Expand All @@ -73,6 +63,6 @@ TAGS: $(shell git ls-files systemd/*.[ch])

shell:
# we change the directory because python insists on adding $CWD to path
(cd $(builddir) && $(PYTHON))
(cd $(BUILD_DIR) && $(PYTHON))
Copy link
Member

@keszybz keszybz Jun 28, 2025

Choose a reason for hiding this comment

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

The parens around this command can be dropped. I'm not sure why they were added in the first place.


.PHONY: build install dist sign upload clean distclean TAGS doc doc-sync shell
100 changes: 100 additions & 0 deletions meson.build
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
project(
'python-systemd',
'c',
version: '236',
license: 'LGPL-2.1-or-later',
default_options: ['warning_level=2', 'c_std=c99'],
)

# Dependencies
python = import('python').find_installation('python3', pure: false)
python_dep = python.dependency()

# Try to find libsystemd
foreach name : [
'libsystemd',
'libsystemd-journal',
'libsystemd-daemon',
'libsystemd-id128',
'libsystemd-login',
Copy link
Member

@keszybz keszybz Jun 28, 2025

Choose a reason for hiding this comment

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

Please drop those compat names. systemd-230 (released 2016-05-21) finally removed them, and they were deprecated since 209 (released 2014-02-20). Then we don't need the loop and things are faster and simpler.

Copy link
Contributor Author

@hongquan hongquan Jun 30, 2025

Choose a reason for hiding this comment

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

Noted!

]
libsystemd_dep = dependency(name, required: false)
if libsystemd_dep.found()
break
endif
endforeach

# Common compile arguments matching setup.py
common_c_args = [
'-std=c99',
Copy link
Member

@keszybz keszybz Jun 28, 2025

Choose a reason for hiding this comment

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

We have 'c_std=c99' in the global options, so this seems duplicated.

EDIT: yeah, the build commands look like this:
ccache cc -Isrc/systemd/id128.cpython-313-x86_64-linux-gnu.so.p -Isrc/systemd -I../src/systemd -I/usr/include/python3.13 -fvisibility=hidden -fdiagnostics-color=always -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wextra -std=c99 -O0 -g -fPIC -std=c99 -Werror=implicit-function-declaration '-DPACKAGE_VERSION="236"' -DLIBSYSTEMD_VERSION=258 -MD -MQ src/systemd/id128.cpython-313-x86_64-linux-gnu.so.p/id128.c.o -MF src/systemd/id128.cpython-313-x86_64-linux-gnu.so.p/id128.c.o.d -o src/systemd/id128.cpython-313-x86_64-linux-gnu.so.p/id128.c.o -c ../src/systemd/id128.c

-std=c99 is inserted twice.

'-Werror=implicit-function-declaration',
Copy link
Member

@keszybz keszybz Jun 28, 2025

Choose a reason for hiding this comment

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

gcc.1 says

This warning is enabled by default, as an error, in C99

So it can be dropped since we use c_std=c99.

'-DPACKAGE_VERSION="' + meson.project_version() + '"',
'-DLIBSYSTEMD_VERSION=' + libsystemd_dep.version(),
]

# Build _journal extension module
python.extension_module(
'_journal',
['systemd/_journal.c', 'systemd/pyutil.c'],
dependencies: [libsystemd_dep],
c_args: common_c_args,
install: true,
subdir: 'systemd',
)

# Build _reader extension module
python.extension_module(
'_reader',
['systemd/_reader.c', 'systemd/pyutil.c', 'systemd/strv.c'],
dependencies: [libsystemd_dep],
c_args: common_c_args,
install: true,
subdir: 'systemd',
)

# Build _daemon extension module
python.extension_module(
'_daemon',
['systemd/_daemon.c', 'systemd/pyutil.c', 'systemd/util.c'],
dependencies: [libsystemd_dep],
c_args: common_c_args,
install: true,
subdir: 'systemd',
)

# Build id128 extension module
python.extension_module(
'id128',
['systemd/id128.c', 'systemd/pyutil.c'],
dependencies: [libsystemd_dep],
c_args: common_c_args,
install: true,
subdir: 'systemd',
)

# Build login extension module
python.extension_module(
'login',
['systemd/login.c', 'systemd/pyutil.c', 'systemd/strv.c'],
dependencies: [libsystemd_dep],
c_args: common_c_args,
install: true,
subdir: 'systemd',
)

# Install Python modules (matching py_modules in setup.py)
Copy link
Member

@keszybz keszybz Jun 28, 2025

Choose a reason for hiding this comment

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

This commit removes setup.py, so the comment is immediately outdated.

python.install_sources(
'systemd/__init__.py',
'systemd/journal.py',
'systemd/daemon.py',
subdir: 'systemd',
)

# Install test modules
python.install_sources(
'systemd/test/test_daemon.py',
'systemd/test/test_journal.py',
'systemd/test/test_login.py',
'systemd/test/test_id128.py',
subdir: 'systemd/test',
)
Loading