Refactor /mnt/pmbootstrap-* to have all these directories as subdirectories of /mnt/pmbootstrap. Fix that these dirs did not get removed when installation was done. Add one more dir in /mnt/pmbootstrap/ for using sccache with rust, similar to how we already use ccache for C/C++ code. It speeds up subsequent builds greatly for native builds, and for native scripts built during cross builds. sccache currently doesn't cache compiler output if --sysroot is used so it doesn't yet work for actual cross compiling and we just don't use it there yet (see pmaports MR). But it could probably be done in the future. The last patch removes an obsolete test for rust + crossdirect stuff. Oliver Smith (5): chroot: /mnt/pmbootstrap-* -> /mnt/pmbootstrap/* install: remove /mnt/pmbootstrap at the end Cosmetic: build: add comment about rust deps build: use sccache for rust test/test_crossdirect: remove pmb/build/_package.py | 11 +++++- pmb/build/init.py | 7 +++- pmb/chroot/__init__.py | 2 +- pmb/chroot/apk.py | 4 +- pmb/chroot/mount.py | 15 +++++++ pmb/chroot/shutdown.py | 14 ++++++- pmb/config/__init__.py | 33 +++++++++------- pmb/helpers/repo.py | 6 +-- pmb/install/_install.py | 5 ++- pmb/netboot/__init__.py | 2 +- test/test_build_package.py | 2 + test/test_chroot_mount.py | 40 +++++++++++++++++++ test/test_crossdirect.py | 81 -------------------------------------- test/test_helpers_repo.py | 4 +- 14 files changed, 115 insertions(+), 111 deletions(-) create mode 100644 test/test_chroot_mount.py delete mode 100644 test/test_crossdirect.py -- 2.41.0
I don't have much experience reviewing pmbootstrap things, but this looks like a clear improvement, much cleaner. I'll see if I have time to test in the following days. Until then Reviewed-by: Pablo Correa Gómez <ablocorrea@hotmail.com> El Sun, 06-08-2023 a las 20:43 +0200, Oliver Smith escribió:
El Sun, 06-08-2023 a las 20:43 +0200, Oliver Smith escribió:
On Sonntag, 6. August 2023 20:43:30 CEST Oliver Smith wrote: I trust you that this comment is correct ;) Reviewed-by: Luca Weiss <luca@z3ntu.xyz>
pmbootstrap/patches/.build.yml: SUCCESS in 13m38s [sccache for rust, refactor /mnt/pmb][0] from [Oliver Smith][1] [0]: https://lists.sr.ht/~postmarketos/pmbootstrap-devel/patches/43386 [1]: mailto:ollieparanoid@postmarketos.org ✓ #1036328 SUCCESS pmbootstrap/patches/.build.yml https://builds.sr.ht/~postmarketos/job/1036328
Copy & paste the following snippet into your terminal to import this patchset into git:
curl -s https://lists.sr.ht/~postmarketos/pmbootstrap-devel/patches/43386/mbox | git am -3Learn more about email & git
Have one /mnt/pmbootstrap directory with subdirectories, instead of several /mnt/pmbootstrap-* directories.
Feels like there could be some fallout where currently "mkdir" works because / mnt already exists, but the new subdirectories might need "mkdir -p". To be clear, I didn't check any code using this, so could very well all be fine.pmb.helpers.mount.bind uses mkdir -p, and I've verified that it works. Thanks for going through the series and reviewing!Reviewed-by: Luca Weiss <luca@z3ntu.xyz>
--- pmb/build/_package.py | 2 +- pmb/build/init.py | 2 +- pmb/chroot/apk.py | 4 ++-- pmb/config/__init__.py | 30 +++++++++++++++--------------- pmb/helpers/repo.py | 6 +++--- pmb/install/_install.py | 4 ++-- pmb/netboot/__init__.py | 2 +- test/test_helpers_repo.py | 4 ++-- 8 files changed, 27 insertions(+), 27 deletions(-) diff --git a/pmb/build/_package.py b/pmb/build/_package.py index 884c931f..c5eb151a 100644 --- a/pmb/build/_package.py +++ b/pmb/build/_package.py @@ -260,7 +260,7 @@ def override_source(args, apkbuild, pkgver, src, suffix="native"): return # Mount source in chroot - mount_path = "/mnt/pmbootstrap-source-override/" + mount_path = "/mnt/pmbootstrap/source-override/" mount_path_outside = args.work + "/chroot_" + suffix + mount_path pmb.helpers.mount.bind(args, src, mount_path_outside, umount=True) diff --git a/pmb/build/init.py b/pmb/build/init.py index b56c6c2c..f10ae73c 100644 --- a/pmb/build/init.py +++ b/pmb/build/init.py @@ -54,7 +54,7 @@ def init(args, suffix="native"): # Copy package signing key to /etc/apk/keys for key in glob.glob(chroot + - "/mnt/pmbootstrap-abuild-config/*.pub"): + "/mnt/pmbootstrap/abuild-config/*.pub"): key = key[len(chroot):] pmb.chroot.root(args, ["cp", key, "/etc/apk/keys/"], suffix) diff --git a/pmb/chroot/apk.py b/pmb/chroot/apk.py index 926f92c1..aa190e1e 100644 --- a/pmb/chroot/apk.py +++ b/pmb/chroot/apk.py @@ -143,7 +143,7 @@ def packages_get_locally_built_apks(args, packages, arch):
arch):
:param packages: list of pkgnames :param arch: architecture that the locally built packages should have :returns: list of apk file paths that are valid inside the chroots, e.g. - ["/mnt/pmbootstrap-packages/x86_64/hello-world-1-r6.apk", ...] + ["/mnt/pmbootstrap/packages/x86_64/hello-world-1-r6.apk", ...] """ channel = pmb.config.pmaports.read_config(args)["channel"] ret = [] @@ -157,7 +157,7 @@ def packages_get_locally_built_apks(args, packages, arch): if not os.path.exists(f"{args.work}/packages/{channel}/{arch}/{apk_file}"): continue - ret.append(f"/mnt/pmbootstrap-packages/{arch}/{apk_file}") + ret.append(f"/mnt/pmbootstrap/packages/{arch}/{apk_file}") return ret diff --git a/pmb/config/__init__.py b/pmb/config/__init__.py index d0dac15f..2ded467a 100644 --- a/pmb/config/__init__.py +++ b/pmb/config/__init__.py @@ -210,15 +210,15 @@ chroot_mount_bind = { "/proc": "/proc", "$WORK/cache_apk_$ARCH": "/var/cache/apk", "$WORK/cache_appstream/$ARCH/$CHANNEL": "/mnt/appstream-data", - "$WORK/cache_ccache_$ARCH": "/mnt/pmbootstrap-ccache", + "$WORK/cache_ccache_$ARCH": "/mnt/pmbootstrap/ccache", "$WORK/cache_distfiles": "/var/cache/distfiles", - "$WORK/cache_git": "/mnt/pmbootstrap-git", - "$WORK/cache_go": "/mnt/pmbootstrap-go", - "$WORK/cache_rust": "/mnt/pmbootstrap-rust", - "$WORK/config_abuild": "/mnt/pmbootstrap-abuild-config", + "$WORK/cache_git": "/mnt/pmbootstrap/git", + "$WORK/cache_go": "/mnt/pmbootstrap/go", + "$WORK/cache_rust": "/mnt/pmbootstrap/rust", + "$WORK/config_abuild": "/mnt/pmbootstrap/abuild-config", "$WORK/config_apk_keys": "/etc/apk/keys", - "$WORK/images_netboot": "/mnt/pmbootstrap-netboot", - "$WORK/packages/$CHANNEL": "/mnt/pmbootstrap-packages", + "$WORK/images_netboot": "/mnt/pmbootstrap/netboot", + "$WORK/packages/$CHANNEL": "/mnt/pmbootstrap/packages", } # Building chroots (all chroots, except for the rootfs_ chroot) get symlinks in @@ -234,14 +234,14 @@ chroot_mount_bind = { # rust depends caching described above) and to cache build artifacts (GOCACHE, # similar to ccache). chroot_home_symlinks = { - "/mnt/pmbootstrap-abuild-config": "/home/pmos/.abuild", - "/mnt/pmbootstrap-ccache": "/home/pmos/.ccache", - "/mnt/pmbootstrap-packages": "/home/pmos/packages/pmos", - "/mnt/pmbootstrap-go/gocache": "/home/pmos/.cache/go-build", - "/mnt/pmbootstrap-go/gomodcache": "/home/pmos/go/pkg/mod", - "/mnt/pmbootstrap-rust/registry/index": "/home/pmos/.cargo/registry/index", - "/mnt/pmbootstrap-rust/registry/cache": "/home/pmos/.cargo/registry/cache", - "/mnt/pmbootstrap-rust/git/db": "/home/pmos/.cargo/git/db", + "/mnt/pmbootstrap/abuild-config": "/home/pmos/.abuild", + "/mnt/pmbootstrap/ccache": "/home/pmos/.ccache", + "/mnt/pmbootstrap/packages": "/home/pmos/packages/pmos", + "/mnt/pmbootstrap/go/gocache": "/home/pmos/.cache/go-build", + "/mnt/pmbootstrap/go/gomodcache": "/home/pmos/go/pkg/mod", + "/mnt/pmbootstrap/rust/registry/index": "/home/pmos/.cargo/registry/index", + "/mnt/pmbootstrap/rust/registry/cache": "/home/pmos/.cargo/registry/cache", + "/mnt/pmbootstrap/rust/git/db": "/home/pmos/.cargo/git/db", } # Device nodes to be created in each chroot. Syntax for each entry: diff --git a/pmb/helpers/repo.py b/pmb/helpers/repo.py index cb5847c8..76d09be8 100644 --- a/pmb/helpers/repo.py +++ b/pmb/helpers/repo.py @@ -43,10 +43,10 @@ def hash(url, length=8): def urls(args, user_repository=True, postmarketos_mirror=True, alpine=True): """ Get a list of repository URLs, as they are in /etc/apk/repositories. - :param user_repository: add /mnt/pmbootstrap-packages + :param user_repository: add /mnt/pmbootstrap/packages :param postmarketos_mirror: add postmarketos mirror URLs :param alpine: add alpine mirror URLs - :returns: list of mirror strings, like ["/mnt/pmbootstrap-packages", + :returns: list of mirror strings, like ["/mnt/pmbootstrap/packages", "http://...", ...] """ ret = [] @@ -59,7 +59,7 @@ def urls(args, user_repository=True, postmarketos_mirror=True, alpine=True): # Local user repository (for packages compiled with pmbootstrap) if user_repository: - ret.append("/mnt/pmbootstrap-packages") + ret.append("/mnt/pmbootstrap/packages") # Upstream postmarketOS binary repository if postmarketos_mirror: diff --git a/pmb/install/_install.py b/pmb/install/_install.py index 652434ae..4b7ff47c 100644 --- a/pmb/install/_install.py +++ b/pmb/install/_install.py @@ -165,7 +165,7 @@ def configure_apk(args): """ Copy over all official keys, and the keys used to compile local packages (unless --no-local-pkgs is set). Then copy the corresponding APKINDEX files - and remove the /mnt/pmbootstrap-packages repository. + and remove the /mnt/pmbootstrap/packages repository. """ # Official keys pattern = f"{pmb.config.apk_keys_path}/*.pub" @@ -187,7 +187,7 @@ def configure_apk(args): pmb.helpers.run.root(args, ["cp", f, rootfs + "/var/cache/apk/"]) # Disable pmbootstrap repository - pmb.helpers.run.root(args, ["sed", "-i", r"/\/mnt\/pmbootstrap-packages/d", + pmb.helpers.run.root(args, ["sed", "-i", r"/\/mnt\/pmbootstrap\/packages/d", rootfs + "/etc/apk/repositories"]) pmb.helpers.run.user(args, ["cat", rootfs + "/etc/apk/repositories"]) diff --git a/pmb/netboot/__init__.py b/pmb/netboot/__init__.py index 1cd4904c..c05449fc 100644 --- a/pmb/netboot/__init__.py +++ b/pmb/netboot/__init__.py @@ -20,7 +20,7 @@ def start_nbd_server(args, ip="172.16.42.2", port=9999): chroot = f"{args.work}/chroot_native" - rootfs_path = f"/mnt/pmbootstrap-netboot/{args.device}.img" + rootfs_path = f"/mnt/pmbootstrap/netboot/{args.device}.img" if not os.path.exists(chroot + rootfs_path) or args.replace: rootfs_path2 = f"/home/pmos/rootfs/{args.device}.img" if not os.path.exists(chroot + rootfs_path2): diff --git a/test/test_helpers_repo.py b/test/test_helpers_repo.py index 1589378d..0b180ad2 100644 --- a/test/test_helpers_repo.py +++ b/test/test_helpers_repo.py @@ -51,7 +51,7 @@ def test_urls(args, monkeypatch): monkeypatch.setattr(pmb.config.pmaports, "read_config", read_config) # Channel: v20.05 - assert func(args) == ["/mnt/pmbootstrap-packages", + assert func(args) == ["/mnt/pmbootstrap/packages", "http://localhost/pmos1/v20.05", "http://localhost/pmos2/v20.05", "http://localhost/alpine/v3.11/main", @@ -59,7 +59,7 @@ def test_urls(args, monkeypatch): # Channel: edge (has Alpine's testing) channel = "edge" - assert func(args) == ["/mnt/pmbootstrap-packages", + assert func(args) == ["/mnt/pmbootstrap/packages", "http://localhost/pmos1/master", "http://localhost/pmos2/master", "http://localhost/alpine/edge/main", -- 2.41.0
Pablo Correa Gomez <pabloyoyoista@postmarketos.org>I don't have much experience reviewing pmbootstrap things, but this looks like a clear improvement, much cleaner. I'll see if I have time to test in the following days. Until then Reviewed-by: Pablo Correa Gómez <ablocorrea@hotmail.com> El Sun, 06-08-2023 a las 20:43 +0200, Oliver Smith escribió:
Don't include the /mnt/pmbootstrap directories in the images created with "pmbootstrap install".
Don't we have some pmbootstrap/pmaports issues that get resolved here? I swear we've had some reports about this.Ah yes! https://gitlab.com/postmarketOS/pmbootstrap/-/issues/2054Anyways: Reviewed-by: Luca Weiss <luca@z3ntu.xyz>
--- pmb/chroot/__init__.py | 2 +- pmb/chroot/mount.py | 15 +++++++++++++++ pmb/config/__init__.py | 1 + pmb/install/_install.py | 1 + test/test_chroot_mount.py | 40 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 test/test_chroot_mount.py diff --git a/pmb/chroot/__init__.py b/pmb/chroot/__init__.py index 01ff116b..01b50a01 100644 --- a/pmb/chroot/__init__.py +++ b/pmb/chroot/__init__.py @@ -1,7 +1,7 @@ # Copyright 2023 Oliver Smith # SPDX-License-Identifier: GPL-3.0-or-later from pmb.chroot.init import init, init_keys -from pmb.chroot.mount import mount, mount_native_into_foreign +from pmb.chroot.mount import mount, mount_native_into_foreign, remove_mnt_pmbootstrap from pmb.chroot.root import root from pmb.chroot.user import user from pmb.chroot.user import exists as user_exists diff --git a/pmb/chroot/mount.py b/pmb/chroot/mount.py index 65f00a7b..45053c0d 100644 --- a/pmb/chroot/mount.py +++ b/pmb/chroot/mount.py @@ -106,3 +106,18 @@ def mount_native_into_foreign(args, suffix): if not os.path.lexists(musl_link): pmb.helpers.run.root(args, ["ln", "-s", "/native/lib/" + musl, musl_link]) + +def remove_mnt_pmbootstrap(args, suffix): + """ Safely remove /mnt/pmbootstrap directories from the chroot, without + running rm -r as root and potentially removing data inside the + mountpoint in case it was still mounted (bug in pmbootstrap, or user + ran pmbootstrap 2x in parallel). This is similar to running 'rm -r -d', + but we don't assume that the host's rm has the -d flag (busybox does + not). """ + mnt_dir = f"{args.work}/chroot_{suffix}/mnt/pmbootstrap" + + if not os.path.exists(mnt_dir): + return + + for path in glob.glob(f"{mnt_dir}/*") + [mnt_dir]: + pmb.helpers.run.root(args, ["rmdir", path]) diff --git a/pmb/config/__init__.py b/pmb/config/__init__.py index 2ded467a..12be7b1a 100644 --- a/pmb/config/__init__.py +++ b/pmb/config/__init__.py @@ -206,6 +206,7 @@ chroot_host_path = os.environ["PATH"] + ":/usr/sbin/" # $WORK gets replaced with args.work # $ARCH gets replaced with the chroot architecture (eg. x86_64, armhf) # $CHANNEL gets replaced with the release channel (e.g. edge, v21.03) +# Use no more than one dir after /mnt/pmbootstrap, see remove_mnt_pmbootstrap.
Pablo Correa Gomez <pabloyoyoista@postmarketos.org>Looks like this comment is misplaced? Otherwise, that's a nitpick. Caveats apply to my knowledge of pmbootstrap, but otherwise. Reviewed-by: Pablo Correa Gómez <ablocorrea@hotmail.com>
chroot_mount_bind = { "/proc": "/proc", "$WORK/cache_apk_$ARCH": "/var/cache/apk", diff --git a/pmb/install/_install.py b/pmb/install/_install.py index 4b7ff47c..98a9127e 100644 --- a/pmb/install/_install.py +++ b/pmb/install/_install.py @@ -816,6 +816,7 @@ def install_system_image(args, size_reserve, suffix, step, steps, # Clean up after running mkinitfs in chroot pmb.helpers.mount.umount_all(args, f"{args.work}/chroot_{suffix}") pmb.helpers.run.root(args, ["rm", f"{args.work}/chroot_{suffix}/in-pmbootstrap"]) + pmb.chroot.remove_mnt_pmbootstrap(args, suffix) # Just copy all the files logging.info(f"*** ({step + 1}/{steps}) FILL INSTALL BLOCKDEVICE ***") diff --git a/test/test_chroot_mount.py b/test/test_chroot_mount.py new file mode 100644 index 00000000..2f4b66db --- /dev/null +++ b/test/test_chroot_mount.py @@ -0,0 +1,40 @@ +# Copyright 2023 Oliver Smith +# SPDX-License-Identifier: GPL-3.0-or-later +""" Test pmb/chroot/mount.py """ +import os +import pytest +import sys + +import pmb_test # noqa +import pmb.chroot + + +@pytest.fixture +def args(tmpdir, request): + import pmb.parse + sys.argv = ["pmbootstrap", "init"] + args = pmb.parse.arguments() + args.log = args.work + "/log_testsuite.txt" + pmb.helpers.logging.init(args) + request.addfinalizer(pmb.helpers.logging.logfd.close) + return args + + +def test_chroot_mount(args): + suffix = "native" + mnt_dir = f"{args.work}/chroot_native/mnt/pmbootstrap" + + # Run something in the chroot to have the dirs created + pmb.chroot.root(args, ["true"]) + assert os.path.exists(mnt_dir) + assert os.path.exists(f"{mnt_dir}/packages") + + # Umount everything, like in pmb.install.install_system_image + pmb.helpers.mount.umount_all(args, f"{args.work}/chroot_{suffix}") + + # Remove all /mnt/pmbootstrap dirs + pmb.chroot.remove_mnt_pmbootstrap(args, suffix) + assert not os.path.exists(mnt_dir) + + # Run again: it should not crash + pmb.chroot.remove_mnt_pmbootstrap(args, suffix) -- 2.41.0
Pablo Correa Gomez <pabloyoyoista@postmarketos.org>El Sun, 06-08-2023 a las 20:43 +0200, Oliver Smith escribió:
--- pmb/build/init.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pmb/build/init.py b/pmb/build/init.py index f10ae73c..38bfae5f 100644 --- a/pmb/build/init.py +++ b/pmb/build/init.py @@ -108,6 +108,9 @@ def init_compiler(args, depends, cross, arch): if cross == "crossdirect": cross_pkgs += ["crossdirect"] if "rust" in depends or "cargo" in depends: + # crossdirect for rust installs all build dependencies in the + # native chroot too, as some of them can be required for building + # native macros / build scripts cross_pkgs += depends pmb.chroot.apk.install(args, cross_pkgs) -- 2.41.0
On Sonntag, 6. August 2023 20:43:30 CEST Oliver Smith wrote: I trust you that this comment is correct ;) Reviewed-by: Luca Weiss <luca@z3ntu.xyz>
Cache the compiler output of rust code with sccache, like we use ccache for c code. I've considered using sccache to completely replace ccache since it can cache output of C/C++ code too. But let's not do it for now since ccache doesn't need to run a daemon in the background that needs to be stopped when shutting down / zapping. Also it would need more refactoring.
Sounds like a decent idea. Any idea about the performance difference vs ccache for C/C++ code? Killing the daemon is already implemented so that's not an obstacle anymore.To my surprise, sccache might be slower: https://github.com/mozilla/sccache/issues/160It's slightly annoying because it takes longer. And currently you need to check out a branch on the same channel in pmaports (master, v23.06 etc., I'd like to get that changed). But yes, if sccache was much faster than ccache it would be worth it.Also if this is done, "pmbootstrap stats" would need adjustment. Or maybe we can already integrate sccache stats into there?I wonder if we should retire "pmbootstrap stats", does anybody use it? It's the same as "pmbootstrap chroot -- ccache -s", or "sscache -s" for sccache. It was useful when implementing the ccache logic very early on, but afterwards I haven't really used it anymore. I just wrote a test patch to also output sccache -s if sccache is installed, but that results in a very long listing and I imagine usually one is interested in either ccache or sccache. How about we hide the option in "pmbootstrap -h" output, but keep the functionality as-is just in case somebody is still using it? I'll merge this now, but it could be done in a future patch.Reviewed-by: Luca Weiss <luca@z3ntu.xyz>
--- pmb/build/_package.py | 9 ++++++++- pmb/build/init.py | 2 ++ pmb/chroot/shutdown.py | 14 +++++++++++++- pmb/config/__init__.py | 8 +++++--- test/test_build_package.py | 2 ++ 5 files changed, 30 insertions(+), 5 deletions(-) diff --git a/pmb/build/_package.py b/pmb/build/_package.py index c5eb151a..486aa127 100644 --- a/pmb/build/_package.py +++ b/pmb/build/_package.py @@ -213,7 +213,10 @@ def init_buildenv(args, apkbuild, arch, strict=False, force=False, cross=None, if not skip_init_buildenv: pmb.build.init(args, suffix) pmb.build.other.configure_abuild(args, suffix) - pmb.build.other.configure_ccache(args, suffix) + if args.ccache: + pmb.build.other.configure_ccache(args, suffix)
These 3 lines feel like a tiny bugfix in itself but totally find to include here.
+ if "rust" in depends or "cargo" in depends: + pmb.chroot.apk.install(args, ["sccache"], suffix) if not strict and "pmb:strict" not in apkbuild["options"] and len(depends): pmb.chroot.apk.install(args, depends, suffix) if src: @@ -400,6 +403,10 @@ def run_abuild(args, apkbuild, arch, strict=False, force=False, cross=None, if not args.ccache: env["CCACHE_DISABLE"] = "1" + # Use sccache without crossdirect (crossdirect uses it via rustc.sh) + if args.ccache and cross != "crossdirect": + env["RUSTC_WRAPPER"] = "/usr/bin/sccache" + # Cache binary objects from go in this path (like ccache) env["GOCACHE"] = "/home/pmos/.cache/go-build" diff --git a/pmb/build/init.py b/pmb/build/init.py index 38bfae5f..5de698bb 100644 --- a/pmb/build/init.py +++ b/pmb/build/init.py @@ -108,6 +108,8 @@ def init_compiler(args, depends, cross, arch): if cross == "crossdirect": cross_pkgs += ["crossdirect"] if "rust" in depends or "cargo" in depends: + if args.ccache: + cross_pkgs += ["sccache"] # crossdirect for rust installs all build dependencies in the # native chroot too, as some of them can be required for building # native macros / build scripts diff --git a/pmb/chroot/shutdown.py b/pmb/chroot/shutdown.py index 23d115ad..20f90604 100644 --- a/pmb/chroot/shutdown.py +++ b/pmb/chroot/shutdown.py @@ -22,6 +22,17 @@ def kill_adb(args): pmb.chroot.root(args, ["adb", "-P", str(port), "kill-server"]) +def kill_sccache(args): + """ + Kill sccache daemon if it's running. Unlike ccache it automatically spawns + a daemon when you call it and exits after some time of inactivity. + """ + port = 4226 + with closing(socket.socket(socket.AF_INET, socket.SOCK_STREAM)) as sock: + if sock.connect_ex(("127.0.0.1", port)) == 0: + pmb.chroot.root(args, ["sccache", "--stop-server"])
Couldn't we unconditionally call "sccache --stop-server"? The code feels a bit overkill just to kill this. Otherwise maybe a nice pkill on the process name?
+ + def shutdown_cryptsetup_device(args, name): """ :param name: cryptsetup device name, usually "pm_crypt" in pmbootstrap @@ -48,8 +59,9 @@ def shutdown_cryptsetup_device(args, name): def shutdown(args, only_install_related=False): - # Stop adb server + # Stop daemons kill_adb(args) + kill_sccache(args) # Umount installation-related paths (order is important!) pmb.helpers.mount.umount_all(args, args.work + diff --git a/pmb/config/__init__.py b/pmb/config/__init__.py index 12be7b1a..098b86ea 100644 --- a/pmb/config/__init__.py +++ b/pmb/config/__init__.py @@ -218,6 +218,7 @@ chroot_mount_bind = { "$WORK/cache_rust": "/mnt/pmbootstrap/rust", "$WORK/config_abuild": "/mnt/pmbootstrap/abuild-config", "$WORK/config_apk_keys": "/etc/apk/keys", + "$WORK/cache_sccache": "/mnt/pmbootstrap/sccache", "$WORK/images_netboot": "/mnt/pmbootstrap/netboot", "$WORK/packages/$CHANNEL": "/mnt/pmbootstrap/packages", } @@ -237,12 +238,13 @@ chroot_mount_bind = { chroot_home_symlinks = { "/mnt/pmbootstrap/abuild-config": "/home/pmos/.abuild", "/mnt/pmbootstrap/ccache": "/home/pmos/.ccache", - "/mnt/pmbootstrap/packages": "/home/pmos/packages/pmos", "/mnt/pmbootstrap/go/gocache": "/home/pmos/.cache/go-build", "/mnt/pmbootstrap/go/gomodcache": "/home/pmos/go/pkg/mod", - "/mnt/pmbootstrap/rust/registry/index": "/home/pmos/.cargo/registry/index", - "/mnt/pmbootstrap/rust/registry/cache": "/home/pmos/.cargo/registry/cache", + "/mnt/pmbootstrap/packages": "/home/pmos/packages/pmos", "/mnt/pmbootstrap/rust/git/db": "/home/pmos/.cargo/git/db", + "/mnt/pmbootstrap/rust/registry/cache": "/home/pmos/.cargo/registry/cache", + "/mnt/pmbootstrap/rust/registry/index": "/home/pmos/.cargo/registry/index", + "/mnt/pmbootstrap/sccache": "/home/pmos/.cache/sccache", } # Device nodes to be created in each chroot. Syntax for each entry: diff --git a/test/test_build_package.py b/test/test_build_package.py index 800ff609..f9cc73c5 100644 --- a/test/test_build_package.py +++ b/test/test_build_package.py @@ -274,6 +274,7 @@ def test_run_abuild(args, monkeypatch): output = "armhf/test-1-r2.apk" env = {"CARCH": "armhf", "GOCACHE": "/home/pmos/.cache/go-build", + "RUSTC_WRAPPER": "/usr/bin/sccache", "SUDO_APK": "abuild-apk --no-progress"} cmd = ["abuild", "-D", "postmarketOS", "-d"] assert func(args, apkbuild, "armhf") == (output, cmd, env) @@ -285,6 +286,7 @@ def test_run_abuild(args, monkeypatch): # cross=native env = {"CARCH": "armhf", "GOCACHE": "/home/pmos/.cache/go-build", + "RUSTC_WRAPPER": "/usr/bin/sccache", "SUDO_APK": "abuild-apk --no-progress", "CROSS_COMPILE": "armv6-alpine-linux-musleabihf-", "CC": "armv6-alpine-linux-musleabihf-gcc"} -- 2.41.0
Remove test_crossdirect_rust, the only test in test_crossdirect.py. The behavior that was tested here with all the "mv /usr/bin/rustc" commands was specific to version 4 and isn't valid anymore since version 5 (pmaports MR 4234). The previous behavior was that crossdirect always tried to run rustc from /native, and if it failed, it would fall back to using qemu. The new behavior is that it runs rustc from /native only when it expects it to succeed, and runs rustc via qemu otherwise, without attempting /native first. As the logic to decide whether to use native rustc or not is now done beforehand, it does not do the fallback to qemu if native fails. Note that the test hasn't been failing because we switch to the v23.06 channel and therefore use the older crossdirect version.
Reviewed-by: Luca Weiss <luca@z3ntu.xyz>
--- test/test_crossdirect.py | 81 ---------------------------------------- 1 file changed, 81 deletions(-) delete mode 100644 test/test_crossdirect.py diff --git a/test/test_crossdirect.py b/test/test_crossdirect.py deleted file mode 100644 index 5212a3a1..00000000 --- a/test/test_crossdirect.py @@ -1,81 +0,0 @@ -# Copyright 2023 Oliver Smith -# SPDX-License-Identifier: GPL-3.0-or-later -import pytest -import sys - -import pmb_test # noqa -import pmb.chroot.apk_static -import pmb.config.pmaports -import pmb.parse.apkindex -import pmb.helpers.logging -import pmb.helpers.run -import pmb.parse.bootimg - - -@pytest.fixture -def args(request): - import pmb.parse - sys.argv = ["pmbootstrap.py", "chroot"] - args = pmb.parse.arguments() - args.log = args.work + "/log_testsuite.txt" - pmb.helpers.logging.init(args) - request.addfinalizer(pmb.helpers.logging.logfd.close) - return args - - -def pmbootstrap_run(args, parameters, check=True): - """Execute pmbootstrap.py with a test pmbootstrap.conf.""" - return pmb.helpers.run.user(args, ["./pmbootstrap.py"] + parameters, - working_dir=pmb.config.pmb_src, - check=check) - - -def test_crossdirect_rust(args): - """ Set up buildroot_armv7 chroot for building, but remove /usr/bin/rustc. - Build hello-world-rust for armv7, to verify that it uses - /native/usr/bin/rustc instead of /usr/bin/rustc. The package has a - check() function, which makes sure that the built program is actually - working. """ - pmbootstrap_run(args, ["-y", "zap"]) - - # Remember previously selected device - cfg = pmb.config.load(args) - old_device = cfg['pmbootstrap']['device'] - - try: - # First, switch to device that is known to exist on all channels, - # such as qemu-amd64. Currently selected device may not exist in - # stable branch! - cfg['pmbootstrap']['device'] = 'qemu-amd64' - pmb.config.save(args, cfg) - - # Switch to "v20.05" channel, as a stable release of alpine is more - # likely to have the same rustc version across various architectures. - # If armv7/x86_64 have a different rustc version, this test will fail: - # 'found crate `std` compiled by an incompatible version of rustc' - pmb.config.pmaports.switch_to_channel_branch(args, "v23.06") - - pmbootstrap_run(args, ["build_init", "-barmv7"]) - pmbootstrap_run(args, ["chroot", "--add=rust", "-barmv7", "--", - "mv", "/usr/bin/rustc", "/usr/bin/rustc_"]) - pmbootstrap_run(args, ["build", "hello-world-rust", "--arch=armv7", - "--force"]) - # Make /native/usr/bin/rustc unusable too, to make the build fail - pmbootstrap_run(args, ["chroot", "--", "rm", "/usr/bin/rustc"]) - assert pmbootstrap_run(args, ["build", "hello-world-rust", - "--arch=armv7", "--force"], - check=False) == 1 - - # Make /usr/bin/rustc usable again, to test fallback with qemu - pmbootstrap_run(args, ["chroot", "-barmv7", "--", - "mv", "/usr/bin/rustc_", "/usr/bin/rustc"]) - pmbootstrap_run(args, ["build", "hello-world-rust", "--arch=armv7", - "--force"]) - finally: - # Clean up - pmb.config.pmaports.switch_to_channel_branch(args, "edge") - pmbootstrap_run(args, ["-y", "zap"]) - - # Restore previously selected device - cfg['pmbootstrap']['device'] = old_device - pmb.config.save(args, cfg) -- 2.41.0
builds.sr.ht <builds@sr.ht>pmbootstrap/patches/.build.yml: SUCCESS in 13m38s [sccache for rust, refactor /mnt/pmb][0] from [Oliver Smith][1] [0]: https://lists.sr.ht/~postmarketos/pmbootstrap-devel/patches/43386 [1]: mailto:ollieparanoid@postmarketos.org ✓ #1036328 SUCCESS pmbootstrap/patches/.build.yml https://builds.sr.ht/~postmarketos/job/1036328