Skip to content

Commit

Permalink
Modernize build system
Browse files Browse the repository at this point in the history
Remove `requirements.txt`; runtime dependencies are inlined in
`install_requires` in `setup.py` and build dependencies are moved to
`pyproject.toml`.

This allows building with `uv` to succeed.
  • Loading branch information
tamird committed Oct 8, 2024
1 parent 6ba970f commit 05f9481
Show file tree
Hide file tree
Showing 8 changed files with 64 additions and 108 deletions.
21 changes: 15 additions & 6 deletions .github/workflows/pylint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,26 @@ jobs:
matrix:
python-version: ["3.8", "3.9", "3.10"]
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v3
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python-version }}
- name: Install dependencies
run: |
set -euxo pipefail
python -m pip install --upgrade pip
pip install -r requirements.txt
pip install pylint
pip install flake8
# Both build and runtime deps. This is the price of using pip.
pip install 'argparse-manpage[setuptools]' argcomplete requests
pip install pylint flake8
- name: Analysing the code with pylint
run: |
python setup.py lint
set -euxo pipefail
pylint vng '**/*.py'
flake8 vng
find . -name '*.py' | xargs flake8
1 change: 0 additions & 1 deletion MANIFEST.in

This file was deleted.

4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ init:
install: install_from_source
install_from_source:
@echo "Version: $(GIT_DESCRIBE)"
BUILD_VIRTME_NG_INIT=1 pip3 install --verbose -r requirements.txt $(INSTALL_ARGS) .
BUILD_VIRTME_NG_INIT=1 pip3 install --verbose $(INSTALL_ARGS) .

install_only_top:
@echo "Version: $(GIT_DESCRIBE)"
BUILD_VIRTME_NG_INIT=0 pip3 install --verbose -r requirements.txt $(INSTALL_ARGS) .
BUILD_VIRTME_NG_INIT=0 pip3 install --verbose $(INSTALL_ARGS) .
11 changes: 6 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,18 +79,19 @@ To install virtme-ng from source you can clone this git repository and build a
standalone virtme-ng running the following commands:
```
$ git clone --recurse-submodules https://github.com/arighi/virtme-ng.git
$ BUILD_VIRTME_NG_INIT=1 pip3 install --verbose -r requirements.txt .
$ BUILD_VIRTME_NG_INIT=1 pip3 install .
```

If you are in Debian/Ubuntu you may need to install the following packages to
build virtme-ng from source properly:
```
$ sudo apt install python3-pip python3-argcomplete flake8 pylint \
cargo rustc qemu-system-x86
$ sudo apt install python3-pip flake8 pylint cargo rustc qemu-system-x86
```

In recent versions of pip3 you may need to specify `--break-system-packages` to
properly install virtme-ng in your system from source.
If you'd prefer to use `uv`:
```
$ BUILD_VIRTME_NG_INIT=1 uv tool install .
```

* Run from source

Expand Down
22 changes: 22 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
[build-system]
requires = [
"argparse-manpage[setuptools]",
"setuptools",

# Runtime dependencies, needed to generate manpages.
"argcomplete",
"requests",
]

[tool.build_manpages]
manpages = [
"""\
man/vng.1\
:pyfile=virtme_ng/run.py\
:function=make_parser\
:author=virtme-ng is written by Andrea Righi <andrea.righi@canonical.com>\
:author=Based on virtme by Andy Lutomirski <luto@kernel.org>\
:manual_title=virtme-ng\
:description=Quickly run kernels inside a virtualized snapshot of your live system\
""",
]
5 changes: 0 additions & 5 deletions requirements.txt

This file was deleted.

97 changes: 15 additions & 82 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,10 @@

import os
import sys
import platform
import sysconfig
from glob import glob
from shutil import which
from subprocess import check_call, CalledProcessError
from setuptools import setup, Command
from subprocess import check_call
from build_manpages import build_manpages, get_build_py_cmd, get_install_cmd
from setuptools import setup
from setuptools.command.build_py import build_py
from setuptools.command.egg_info import egg_info
from virtme_ng.version import get_version_string
Expand Down Expand Up @@ -35,56 +33,6 @@
os.environ['PYTHONPATH'] = sysconfig.get_paths()['purelib']


def is_arm_32bit():
arch = platform.machine()
return arch.startswith("arm") and platform.architecture()[0] == "32bit"


def parse_requirements(filename):
with open(filename, 'r', encoding="utf-8") as file:
lines = file.readlines()
return [line.strip() for line in lines if line.strip() and not line.startswith('#')]


class LintCommand(Command):
description = "Run coding style checks"
user_options = []

def initialize_options(self):
pass

def finalize_options(self):
pass

def run(self):
try:
for cmd in ("flake8", "pylint"):
command = [cmd]
for pattern in (
"vng",
"*.py",
"virtme/*.py",
"virtme/*/*.py",
"virtme_ng/*.py",
):
command += glob(pattern)
check_call(command)
except CalledProcessError:
sys.exit(1)


man_command = f"""
argparse-manpage \
--pyfile ./virtme_ng/run.py --function make_parser \
--prog vng --version v{VERSION} \
--author "virtme-ng is written by Andrea Righi <andrea.righi@canonical.com>" \
--author "Based on virtme by Andy Lutomirski <luto@kernel.org>" \
--project-name virtme-ng --manual-title virtme-ng \
--description "Quickly run kernels inside a virtualized snapshot of your live system" \
--url https://github.com/arighi/virtme-ng > vng.1
"""


class BuildPy(build_py):
def run(self):
print(f"BUILD_VIRTME_NG_INIT: {build_virtme_ng_init}")
Expand All @@ -95,23 +43,6 @@ def run(self):
["strip", "-s", "../virtme/guest/bin/virtme-ng-init"],
cwd="virtme_ng_init",
)
# Generate manpage
if which('argparse-manpage'):
env = os.environ.copy()
env["PYTHONPATH"] = os.path.dirname(os.path.abspath(__file__))
check_call(man_command, shell=True, env=env)

# Generate bash autocompletion scripts
completion_command = ''
if which("register-python-argcomplete"):
completion_command = "register-python-argcomplete"
elif which("register-python-argcomplete3"):
completion_command = "register-python-argcomplete3"
else:
print("ERROR: 'register-python-argcomplete' or 'register-python-argcomplete3' not found.")
sys.exit(1)
check_call(completion_command + ' virtme-ng > virtme-ng-prompt', shell=True)
check_call(completion_command + ' vng > vng-prompt', shell=True)

# Run the rest of virtme-ng build
build_py.run(self)
Expand Down Expand Up @@ -154,26 +85,28 @@ def run(self):

data_files = [
("/etc", ["cfg/virtme-ng.conf"]),
("/usr/share/bash-completion/completions", ["virtme-ng-prompt"]),
("/usr/share/bash-completion/completions", ["vng-prompt"]),

This comment has been minimized.

Copy link
@jubalh

jubalh Oct 15, 2024

Do I understand it right that 1.30 will not install bash completion files anymore?

This comment has been minimized.

Copy link
@arighi

arighi Oct 15, 2024

Owner

Ah, bummer... the bash completion file is useful. It'd be nice to re-introduce it without completely reverting this change. I'll take a look if nobody else wants to look at this. Thanks for noticing it!

This comment has been minimized.

Copy link
@tamird

tamird Oct 15, 2024

Author Contributor

Yeah, I don't exactly understand how packaging bash completions is supposed to work in a hermetic world. The docs in https://pypi.org/project/argcomplete/ suggest that it's up the user to invoke and generate completions.

I'm not a python expert sadly.

This comment has been minimized.

Copy link
@jubalh

jubalh Oct 15, 2024

Speaking of missing files: the virtme-ng.conf is also not installed anymore. Was that one intentional?

This comment has been minimized.

Copy link
@tamird

tamird Oct 15, 2024

Author Contributor

Not exactly, but again: it's not clear what the right thing to do here is in a hermetic world. I believe the python package ecosystem is moving away from packages that install files in arbitrary locations. I'm not sure how config files like this are meant to be provided; perhaps on first run rather than on install? Perhaps to the user's home directory rather than /etc?

This comment has been minimized.

Copy link
@jubalh

jubalh Oct 15, 2024

I'm not very familiar with Python. But all these questions have been answered for other build systems already, right? Usually there is a default location and a way to overwrite it. And usually default system wide configs are placed there during install not first run mostly.
But my main point is: a commit that tries to modernize something would be cleaner if he doesn't change the current behaviour as a sideeffect without any note about it. I believe that behaviour should then be changed in a separate commit or at least be mentioned in the commit message so that it is a conscious decision and not an oversight :)

This comment has been minimized.

Copy link
@tamird

tamird Oct 15, 2024

Author Contributor

I'm not familiar with any build systems that write files into arbitrary places on disk.

Package managers do, but I'm not sure what the relationship between pip and apt is.

This comment has been minimized.

Copy link
@jubalh

jubalh Oct 15, 2024

Aren't cmake, automake and meson installing configuration files?

This comment has been minimized.

Copy link
@tamird

tamird Oct 15, 2024

Author Contributor

Well, no, they are not. It's possible that the way the are packed by your distribution does, but cmake itself does not.

This comment has been minimized.

Copy link
@jubalh

jubalh Oct 15, 2024

There is something missing in the sentence, so I'm not sure I understood it correctly.
But isn't https://cmake.org/cmake/help/book/mastering-cmake/chapter/Install.html what we are talking about?

This comment has been minimized.

Copy link
@tamird

tamird Oct 15, 2024

Author Contributor

Oh, I see. cmake in this case is the analogue of pip, it will do whatever you tell it. It doesn't have opinions.

This comment has been minimized.

Copy link
@jubalh

jubalh Oct 15, 2024

Well yes.. and that's what we want right?

virtme-ng 1.29 installed a config and bash completion file. And now I'm asking here whether this was intentionally removed for 1.30. So far it doesn't look like it from what I understand.

Regarding whether it should install those files or not, I don't see why it shouldn't offer the choice at least. Plenty of software installs shell completions, default config files, themes, docu etc.

This comment has been minimized.

Copy link
@tamird

tamird Oct 15, 2024

Author Contributor

The problem is that the expectation of many people in the python community is that pip install can be done in a virtualenv, and the result of that installation doesn't escape the venv.

If you can find some precedent that describes how people solve this problem in a hermetic pip world, I'd be grateful.

This comment has been minimized.

Copy link
@jubalh

jubalh Oct 15, 2024

Unfortunately I have really no idea about Python at all.

Is this helpful: https://setuptools.pypa.io/en/latest/userguide/datafiles.html ?

This comment has been minimized.

Copy link
@tamird

tamird Oct 15, 2024

Author Contributor

heh, that page includes exactly what I've been saying:

Old packaging installation methods in the Python ecosystem have traditionally allowed installation of “data files”, which are placed in a platform-specific location. However, the most common use case for data files distributed with a package is for use by the package, usually by including the data files inside the package directory.

https://setuptools.pypa.io/en/latest/userguide/datafiles.html#:~:text=Old%20packaging%20installation,package%20directory.

]

if which('argparse-manpage'):
data_files.append(("/usr/share/man/man1", ["vng.1"]))

setup(
name="virtme-ng",
version=VERSION,
author="Andrea Righi",
author_email="andrea.righi@canonical.com",
description="Build and run a kernel inside a virtualized snapshot of your live system",
url="https://git.launchpad.net/~arighi/+git/virtme-ng",
url="https://github.com/arighi/virtme-ng",
license="GPLv2",
long_description=open(
os.path.join(os.path.dirname(__file__), "README.md"), "r", encoding="utf-8"
).read(),
long_description_content_type="text/markdown",
install_requires=parse_requirements('requirements.txt'),
install_requires=[
'argcomplete',
'requests',
# `pkg_resources` is removed in python 3.12, moved to setuptools.
#
# TODO: replace pkg_resources with importlib. # pylint: disable=fixme
'setuptools',
],
entry_points={
"console_scripts": [
"vng = virtme_ng.run:main",
Expand All @@ -184,13 +117,13 @@ def run(self):
]
},
cmdclass={
"build_py": BuildPy,
"build_manpages": build_manpages,
"build_py": get_build_py_cmd(BuildPy),
"install": get_install_cmd(),
"egg_info": EggInfo,
"lint": LintCommand,
},
packages=packages,
package_data={"virtme.guest": package_files},
data_files=data_files,
scripts=[
"bin/virtme-prep-kdir-mods",
],
Expand Down
11 changes: 4 additions & 7 deletions virtme_ng/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,8 @@
)
from select import select
from pathlib import Path
try:
from argcomplete import autocomplete
except ModuleNotFoundError:
def autocomplete(*args, **kwargs):
# pylint: disable=unused-argument
pass

import argcomplete

from virtme.util import SilentError, uname, get_username
from virtme_ng.utils import CONF_FILE, spinner_decorator
Expand Down Expand Up @@ -79,6 +75,7 @@ def make_parser():
"""Main virtme-ng command line parser."""

parser = argparse.ArgumentParser(
prog="vng",
formatter_class=argparse.RawTextHelpFormatter,
description="Build and run kernels inside a virtualized snapshot of your live system",
epilog="""\
Expand Down Expand Up @@ -1245,7 +1242,7 @@ def dump(kern_source, args):

def do_it() -> int:
"""Main body."""
autocomplete(_ARGPARSER)
argcomplete.autocomplete(_ARGPARSER)
args = _ARGPARSER.parse_args()

kern_source = KernelSource()
Expand Down

0 comments on commit 05f9481

Please sign in to comment.