| Message ID | 20221019104745.479166-1-adriaan.schmidt@siemens.com |
|---|---|
| State | RFC, archived |
| Headers | show |
| Series | [RFC] Remove isar-apt and the corresponding lock | expand |
On 19.10.22 12:47, Adriaan Schmidt wrote: > Hi all, > > the isar-apt file lock has been bothering me for a while now, mainly because > it leads to situations where we see many bitbake tasks running in parallel, > but unable to make any progress (deploy_deb, install_builddeps, ...). > > I ran a quick experiment on what would be required to change that, and > came up with this approach: > - isar-apt/ is no longer a repository maintained with reprepro > - each recipe has "its own" directory below isar-apt/ > - there are directories isar-apt/${PN}/in and isar-apt/${PN}/out > - out/ contains the deb packages produced, put there by deploy_deb > - in/ is populated by prepare_build and contains the contents of in/ and out/ > of all DEPENDS (need all the in/ contents due to transitive dependencies) > - then, again in prepare_build, we copy our in/ directory to WORKDIR, and > create an apt repository (actually just a "Packages" file) using dpkg-scanpackages. > > In addition to removing the lock, this also gives us better control > over what goes into a package build, and eliminates effects due to "accidental" > packages in isar-apt. This sounds like a great side-effect. Love it. > > I've implemented a very rough PoC, just to give you an idea. Please don't > look too closely at the code, there are many things missing (e.g. SDKs, > architectures != amd64, ...). But it can build isar-image-base for amd64. > > If we decide that this would be a worthwile change, I will spend some > more time on it. > > Best, > Adriaan best regards raphael
On Wed, Oct 19, 2022 at 12:47:45PM +0200, Adriaan Schmidt wrote: > In addition to removing the lock, this also gives us better control > over what goes into a package build, and eliminates effects due to "accidental" > packages in isar-apt. We are aware of this issue. isar-apt is the "Debian way" deliverable from an Isar-based project, containing the locally-built packages. To implement proper Debian Build-Depends functionality (without duplicating DEPENDS on bitbake level), we need the respective introspection of which packages come from Debian and which ones are built locally. For this, we need to enhance base-apt and isar-apt (and maybe bitbake or some new class) to enable such introspection. [1] and [2] are the beginning of this work (they don't cover all of it today). We've looked at the locking in scope of [2]. We know that isar-apt critical section is too coarse and can be greatly narrowed down. If this is the pain point, we could prioritize that specific issue and provide the respective improvement. We'll check your patch in more detail, but I'm against removing isar-apt altogether. 1. base-apt enhancement: https://patchwork.isar-build.org/project/isar/patch/20220325103226.27033-1-ubely@ilbers.de/ 2. isar-apt enhancement: https://patchwork.isar-build.org/project/isar/patch/20220613070759.16949-1-ubely@ilbers.de/ With kind regards, Baurzhan
Baurzhan Ismagulov, Freitag, 21. Oktober 2022 12:15: > On Wed, Oct 19, 2022 at 12:47:45PM +0200, Adriaan Schmidt wrote: > > In addition to removing the lock, this also gives us better control > > over what goes into a package build, and eliminates effects due to > "accidental" > > packages in isar-apt. > > We are aware of this issue. isar-apt is the "Debian way" deliverable from an > Isar-based project, containing the locally-built packages. To implement > proper > Debian Build-Depends functionality (without duplicating DEPENDS on bitbake > level), we need the respective introspection of which packages come from > Debian > and which ones are built locally. For this, we need to enhance base-apt and > isar-apt (and maybe bitbake or some new class) to enable such introspection. > [1] and [2] are the beginning of this work (they don't cover all of it > today). I don't see a conflict here. I'd say the .deb introspection and more advanced detection of dependencies is an independent issue. > We've looked at the locking in scope of [2]. We know that isar-apt critical > section is too coarse and can be greatly narrowed down. If this is the pain > point, we could prioritize that specific issue and provide the respective > improvement. We'll check your patch in more detail, but I'm against removing > isar-apt altogether. I see the value of isar-apt as a deliverable of a build. But I don't think it's the best tool to transfer packages during build. To transfer between recipes of the same build, this RFC could be an alternative. And to transfer to other builds (re-use packages to speed things up), we have sstate. I had a look at [2], and one problem I see is that packages are only identified by their filename and version, and this does not lead to a rebuild when any of the inputs (sources, recipe) change. If we remove isar-apt as a tool to transfer packages during build, I would definitely bring it back, possibly as a feature of the rootfs or image class (e.g., bitbake my-image -c export-apt-repo). Adriaan > 1. base-apt enhancement: https://patchwork.isar- > build.org/project/isar/patch/20220325103226.27033-1-ubely@ilbers.de/ > 2. isar-apt enhancement: https://patchwork.isar- > build.org/project/isar/patch/20220613070759.16949-1-ubely@ilbers.de/ > > With kind regards, > Baurzhan >
On Mon, 2022-12-12 at 12:36 +0000, Schmidt, Adriaan wrote: > Baurzhan Ismagulov, Freitag, 21. Oktober 2022 12:15: > > On Wed, Oct 19, 2022 at 12:47:45PM +0200, Adriaan Schmidt wrote: > > > In addition to removing the lock, this also gives us better > > > control > > > over what goes into a package build, and eliminates effects due > > > to > > "accidental" > > > packages in isar-apt. > > > > We are aware of this issue. isar-apt is the "Debian way" > > deliverable from an > > Isar-based project, containing the locally-built packages. To > > implement > > proper > > Debian Build-Depends functionality (without duplicating DEPENDS on > > bitbake > > level), we need the respective introspection of which packages come > > from > > Debian > > and which ones are built locally. For this, we need to enhance > > base-apt and > > isar-apt (and maybe bitbake or some new class) to enable such > > introspection. > > [1] and [2] are the beginning of this work (they don't cover all of > > it > > today). > > I don't see a conflict here. I'd say the .deb introspection and more > advanced > detection of dependencies is an independent issue. I also would consider this as two independent aspects. > > > We've looked at the locking in scope of [2]. We know that isar-apt > > critical > > section is too coarse and can be greatly narrowed down. If this is > > the pain > > point, we could prioritize that specific issue and provide the > > respective > > improvement. We'll check your patch in more detail, but I'm against > > removing > > isar-apt altogether. The point is not about removing isar-apt all together, but removing the global state which is shared across the tasks. Having this global state is bad regarding the following aspects: - reproducibility: We inject data that is not covered in any bitbake hash - blocking the scheduler: The access lock reduces the parallelism and also smudge the statistics - no way to remove packages between two invocations of bitbake: This is mainly relevant with local re-builds and pre-built packages where only some should be deployed to the isar-apt. I recently debugged such a case for an hour to understand why not the upstream package of ntpsec is used, but my broken custom-built one (which I removed but still lingered in the isar-apt) I highly vote to further develop this RFC. PS: I cannot access the mentioned links, looks like your patchwork is down (or something is broken with our network). Felix > > I see the value of isar-apt as a deliverable of a build. But I don't > think > it's the best tool to transfer packages during build. To transfer > between > recipes of the same build, this RFC could be an alternative. And to > transfer > to other builds (re-use packages to speed things up), we have sstate. > I had a look at [2], and one problem I see is that packages are only > identified > by their filename and version, and this does not lead to a rebuild > when > any of the inputs (sources, recipe) change. > > If we remove isar-apt as a tool to transfer packages during build, I > would > definitely bring it back, possibly as a feature of the rootfs or > image class > (e.g., bitbake my-image -c export-apt-repo). > > Adriaan > > > 1. base-apt enhancement: https://patchwork.isar- > > build.org/project/isar/patch/20220325103226.27033-1-ubely@ilbers.de > > / > > 2. isar-apt enhancement: https://patchwork.isar- > > build.org/project/isar/patch/20220613070759.16949-1-ubely@ilbers.de > > / > > > > With kind regards, > > Baurzhan > > >
On Tue, Dec 13, 2022 at 03:44:09AM +0000, Moessbauer, Felix wrote: > The point is not about removing isar-apt all together, but removing the > global state which is shared across the tasks. Having this global state > is bad regarding the following aspects: > > - reproducibility: We inject data that is not covered in any bitbake > hash > - blocking the scheduler: The access lock reduces the parallelism and > also smudge the statistics > - no way to remove packages between two invocations of bitbake: > This is mainly relevant with local re-builds and pre-built packages > where only some should be deployed to the isar-apt. I recently debugged > such a case for an hour to understand why not the upstream package of > ntpsec is used, but my broken custom-built one (which I removed but > still lingered in the isar-apt) Thanks for the specific aspects, we will look at that. To my knowledge, at least the scheduler topic should not be an issue after sbuild migration. Reproducibility and removal are technical issues that can be improved, there is nothing inherently broken with the approach. In general, we rely on Debian tooling for building packages and root filesystems. Those, in turn, rely on apt. So, for Debian, apt repos are the transfer mechanism. There are some aspects where Debian, being a distro and not a development framework, shows its limitations for our use cases (version number bumping, etc.). What I'd like to avoid is to create an inherently source-based distribution disparate from Debian that emulates some deliverables at the end; we should evaluate our use cases and look at enhancing Debian workflows if necessary. > PS: I cannot access the mentioned links, looks like your patchwork is > down (or something is broken with our network). Seems to be ok at least according to https://downforeveryoneorjustme.com/patchwork.isar-build.org With kind regards, Baurzhan
diff --git a/meta/classes/dpkg-base.bbclass b/meta/classes/dpkg-base.bbclass index 12d18592..a5ed95b1 100644 --- a/meta/classes/dpkg-base.bbclass +++ b/meta/classes/dpkg-base.bbclass @@ -17,7 +17,7 @@ DEPENDS_append_riscv64 = "${@' crossbuild-essential-riscv64' if d.getVar('ISAR_C DEB_BUILD_PROFILES ?= "" DEB_BUILD_OPTIONS ?= "" -ISAR_APT_REPO ?= "deb [trusted=yes] file:///home/builder/${PN}/isar-apt/${DISTRO}-${DISTRO_ARCH}/apt/${DISTRO} ${DEBDISTRONAME} main" +ISAR_APT_REPO ?= "deb [trusted=yes] file:///home/builder/${PN}/isar-apt ${DEBDISTRONAME} main" python do_adjust_git() { import subprocess @@ -179,7 +179,6 @@ PPS ?= "${@get_package_srcdir(d)}" # Empty do_prepare_build() implementation, to be overwritten if needed do_prepare_build() { - true } addtask prepare_build after do_patch do_transform_template before do_dpkg_build @@ -212,15 +211,26 @@ dpkg_undo_mounts() { sudo rmdir ${BUILDROOT} } +do_prepare_build[cleandirs] += "${REPO_ISAR_DIR}/${DISTRO}/${PN}/in ${WORKDIR}/isar-apt" + do_prepare_build_append() { - # Make a local copy of isar-apt repo that is not affected by other parallel builds - mkdir -p ${WORKDIR}/isar-apt/${DISTRO}-${DISTRO_ARCH} - rm -rf ${WORKDIR}/isar-apt/${DISTRO}-${DISTRO_ARCH}/* - cp -Rl ${REPO_ISAR_DIR} ${WORKDIR}/isar-apt/${DISTRO}-${DISTRO_ARCH} + # in deploy/isar-apt: collect all (transitive) deps in .../in/ + for dep in ${DEPENDS}; do + for f in $(find ${REPO_ISAR_DIR}/${DISTRO}/${dep}/in/ ${REPO_ISAR_DIR}/${DISTRO}/${dep}/out/ -maxdepth 1 -name '*.deb' -print); do + cp -l "$f" ${REPO_ISAR_DIR}/${DISTRO}/${PN}/in/ + done + done + # for the build itself: copy everything into workdir + cd ${WORKDIR}/isar-apt + mkdir pool dists + for f in $(find ${REPO_ISAR_DIR}/${DISTRO}/${PN}/in/ -maxdepth 1 -name '*.deb' -print); do + cp -l "$f" pool/ + done + # make it a deb repo + mkdir -p dists/${DEBDISTRONAME}/main/binary-${DISTRO_ARCH}/ + dpkg-scanpackages pool/ > dists/${DEBDISTRONAME}/main/binary-${DISTRO_ARCH}/Packages } -do_prepare_build[lockfiles] += "${REPO_ISAR_DIR}/isar.lock" - # Placeholder for actual dpkg_runbuild() implementation dpkg_runbuild() { die "This should never be called, overwrite it in your derived class" @@ -282,28 +292,20 @@ do_dpkg_build[depends] = "${SCHROOT_DEP}" CLEANFUNCS += "deb_clean" deb_clean() { - DEBS=$( find ${WORKDIR} -maxdepth 1 -name "*.deb" || [ ! -d ${S} ] ) - if [ -n "${DEBS}" ]; then - for d in ${DEBS}; do - repo_del_package "${REPO_ISAR_DIR}"/"${DISTRO}" \ - "${REPO_ISAR_DB_DIR}"/"${DISTRO}" "${DEBDISTRONAME}" "${d}" - done - fi + rm -rf ${REPO_ISAR_DIR}/${DISTRO}/${PN} } -# the clean function modifies isar-apt -do_clean[lockfiles] = "${REPO_ISAR_DIR}/isar.lock" do_deploy_deb() { - deb_clean - repo_add_packages "${REPO_ISAR_DIR}"/"${DISTRO}" \ - "${REPO_ISAR_DB_DIR}"/"${DISTRO}" "${DEBDISTRONAME}" ${WORKDIR}/*.deb + # place our results in .../out/ + for f in $(find ${WORKDIR} -maxdepth 1 -name '*.deb' -print); do + cp -l "$f" ${REPO_ISAR_DIR}/${DISTRO}/${PN}/out/ + done } addtask deploy_deb after do_dpkg_build before do_build do_deploy_deb[deptask] = "do_deploy_deb" -do_deploy_deb[depends] += "isar-apt:do_cache_config" -do_deploy_deb[lockfiles] = "${REPO_ISAR_DIR}/isar.lock" do_deploy_deb[dirs] = "${S}" +do_deploy_deb[cleandirs] = "${REPO_ISAR_DIR}/${DISTRO}/${PN}/out" python do_devshell() { bb.build.exec_func('schroot_create_configs', d) diff --git a/meta/classes/image-tools-extension.bbclass b/meta/classes/image-tools-extension.bbclass index e13d4a3f..1a516dcf 100644 --- a/meta/classes/image-tools-extension.bbclass +++ b/meta/classes/image-tools-extension.bbclass @@ -11,7 +11,7 @@ IMAGER_INSTALL ??= "" IMAGER_BUILD_DEPS ??= "" DEPENDS += "${IMAGER_BUILD_DEPS}" -do_install_imager_deps[depends] = "${BUILDCHROOT_DEP} isar-apt:do_cache_config" +do_install_imager_deps[depends] = "${BUILDCHROOT_DEP}" do_install_imager_deps[deptask] = "do_deploy_deb" do_install_imager_deps[lockfiles] += "${REPO_ISAR_DIR}/isar.lock" do_install_imager_deps() { diff --git a/meta/classes/rootfs.bbclass b/meta/classes/rootfs.bbclass index bbb5ac0a..036925a4 100644 --- a/meta/classes/rootfs.bbclass +++ b/meta/classes/rootfs.bbclass @@ -78,6 +78,19 @@ BOOTSTRAP_SRC_${ROOTFS_ARCH} = "${DEPLOY_DIR_BOOTSTRAP}/${ROOTFS_DISTRO}-${ROOTF rootfs_prepare[weight] = "25" rootfs_prepare(){ sudo cp -Trpfx '${BOOTSTRAP_SRC}/' '${ROOTFSDIR}' + mkdir -p ${REPO_ISAR_DIR}/${DISTRO}/${PN}/pool + echo "deps: ${DEPENDS}" + for dep in ${DEPENDS}; do + # hackity-hack: for some reason the kernel pkg is in DEPDENDS, even if there's no recipe + # could this be an isar-bug!? + [ "${dep}" = "${KERNEL_IMAGE_PKG}" ] && continue + for f in $(find ${REPO_ISAR_DIR}/${DISTRO}/${dep}/in/ ${REPO_ISAR_DIR}/${DISTRO}/${dep}/out/ -maxdepth 1 -name '*.deb' -print); do + cp -l "$f" ${REPO_ISAR_DIR}/${DISTRO}/${PN}/pool + done + done + # make it a deb repo + mkdir -p ${REPO_ISAR_DIR}/${DISTRO}/${PN}/dists/${DEBDISTRONAME}/main/binary-${DISTRO_ARCH}/ + cd ${REPO_ISAR_DIR}/${DISTRO}/${PN} && dpkg-scanpackages pool/ > dists/${DEBDISTRONAME}/main/binary-${DISTRO_ARCH}/Packages } ROOTFS_CONFIGURE_COMMAND += "rootfs_configure_isar_apt" @@ -87,7 +100,7 @@ rootfs_configure_isar_apt() { set -e mkdir -p '${ROOTFSDIR}/etc/apt/sources.list.d' - echo 'deb [trusted=yes] file:///isar-apt ${DEBDISTRONAME} main' > \ + echo 'deb [trusted=yes] file:///isar-apt/${PN} ${DEBDISTRONAME} main' > \ '${ROOTFSDIR}/etc/apt/sources.list.d/isar-apt.list' mkdir -p '${ROOTFSDIR}/etc/apt/preferences.d' @@ -117,7 +130,6 @@ EOSUDO ROOTFS_INSTALL_COMMAND += "rootfs_install_pkgs_update" rootfs_install_pkgs_update[weight] = "5" -rootfs_install_pkgs_update[isar-apt-lock] = "acquire-before" rootfs_install_pkgs_update() { sudo -E chroot '${ROOTFSDIR}' /usr/bin/apt-get update \ -o Dir::Etc::SourceList="sources.list.d/isar-apt.list" \ @@ -142,7 +154,6 @@ rootfs_import_package_cache() { ROOTFS_INSTALL_COMMAND += "rootfs_install_pkgs_download" rootfs_install_pkgs_download[weight] = "600" -rootfs_install_pkgs_download[isar-apt-lock] = "release-after" rootfs_install_pkgs_download() { sudo -E chroot '${ROOTFSDIR}' \ /usr/bin/apt-get ${ROOTFS_APT_ARGS} --download-only ${ROOTFS_PACKAGES} @@ -195,15 +206,8 @@ python do_rootfs_install() { for cmd in cmds: progress_reporter.next_stage() - - if (d.getVarFlag(cmd, 'isar-apt-lock') or "") == "acquire-before": - lock = bb.utils.lockfile(d.getVar("REPO_ISAR_DIR") + "/isar.lock", - shared=True) - bb.build.exec_func(cmd, d) - if (d.getVarFlag(cmd, 'isar-apt-lock') or "") == "release-after": - bb.utils.unlockfile(lock) progress_reporter.finish() } addtask rootfs_install before do_rootfs_postprocess after do_unpack @@ -294,7 +298,7 @@ python do_rootfs() { } addtask rootfs before do_build -do_rootfs_postprocess[depends] = "base-apt:do_cache isar-apt:do_cache_config" +do_rootfs_postprocess[depends] = "base-apt:do_cache" SSTATETASKS += "do_rootfs_install" SSTATECREATEFUNCS += "rootfs_install_sstate_prepare" @@ -311,7 +315,6 @@ rootfs_install_sstate_prepare() { sudo umount ${WORKDIR}/mnt/rootfs sudo chown $(id -u):$(id -g) rootfs.tar } -do_rootfs_install_sstate_prepare[lockfiles] = "${REPO_ISAR_DIR}/isar.lock" rootfs_install_sstate_finalize() { # this runs in SSTATE_INSTDIR diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf index 98412e02..e7874a2b 100644 --- a/meta/conf/bitbake.conf +++ b/meta/conf/bitbake.conf @@ -88,8 +88,7 @@ TARGET_OS = "isar" PACKAGE_ARCH ?= "${DISTRO_ARCH}" # Isar apt repository paths -REPO_ISAR_DIR = "${DEPLOY_DIR}/isar-apt/${DISTRO}-${DISTRO_ARCH}/apt" -REPO_ISAR_DB_DIR = "${DEPLOY_DIR}/isar-apt/${DISTRO}-${DISTRO_ARCH}/db" +REPO_ISAR_DIR = "${DEPLOY_DIR}/isar-apt" THIRD_PARTY_APT_KEYRING = "/etc/apt/trusted.gpg.d/third_party.gpg" # Base apt repository paths diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc index fdaad61b..4df21d99 100644 --- a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc +++ b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc @@ -276,7 +276,7 @@ do_bootstrap[vardeps] += " \ ${DISTRO_VARS_PREFIX}DISTRO_APT_SOURCES \ " do_bootstrap[dirs] = "${DEPLOY_DIR_BOOTSTRAP}" -do_bootstrap[depends] = "base-apt:do_cache isar-apt:do_cache_config" +do_bootstrap[depends] = "base-apt:do_cache" do_bootstrap() { if [ "${ISAR_ENABLE_COMPAT_ARCH}" = "1" ]; then diff --git a/meta/recipes-devtools/isar-apt/files/distributions.in b/meta/recipes-devtools/isar-apt/files/distributions.in deleted file mode 100644 index 3cf7ea55..00000000 --- a/meta/recipes-devtools/isar-apt/files/distributions.in +++ /dev/null @@ -1,3 +0,0 @@ -Codename: {CODENAME} -Architectures: i386 armhf arm64 amd64 mipsel riscv64 source -Components: main diff --git a/meta/recipes-devtools/isar-apt/isar-apt.bb b/meta/recipes-devtools/isar-apt/isar-apt.bb deleted file mode 100644 index 0458bac7..00000000 --- a/meta/recipes-devtools/isar-apt/isar-apt.bb +++ /dev/null @@ -1,23 +0,0 @@ -# This software is a part of ISAR. -# Copyright (C) 2015-2017 ilbers GmbH -# Copyright (C) 2020 Siemens AG -# -# SPDX-License-Identifier: MIT - -inherit repository - -SRC_URI = "file://distributions.in" - -do_cache_config[stamp-extra-info] = "${DISTRO}" -do_cache_config[lockfiles] = "${REPO_ISAR_DIR}/isar.lock" - -# Generate reprepro config for current distro if it doesn't exist. Once it's -# generated, this task should do nothing. -do_cache_config() { - repo_create "${REPO_ISAR_DIR}"/"${DISTRO}" \ - "${REPO_ISAR_DB_DIR}"/"${DISTRO}" \ - "${DEBDISTRONAME}" \ - "${WORKDIR}/distributions.in" -} - -addtask cache_config after do_unpack before do_build
Hi all, the isar-apt file lock has been bothering me for a while now, mainly because it leads to situations where we see many bitbake tasks running in parallel, but unable to make any progress (deploy_deb, install_builddeps, ...). I ran a quick experiment on what would be required to change that, and came up with this approach: - isar-apt/ is no longer a repository maintained with reprepro - each recipe has "its own" directory below isar-apt/ - there are directories isar-apt/${PN}/in and isar-apt/${PN}/out - out/ contains the deb packages produced, put there by deploy_deb - in/ is populated by prepare_build and contains the contents of in/ and out/ of all DEPENDS (need all the in/ contents due to transitive dependencies) - then, again in prepare_build, we copy our in/ directory to WORKDIR, and create an apt repository (actually just a "Packages" file) using dpkg-scanpackages. In addition to removing the lock, this also gives us better control over what goes into a package build, and eliminates effects due to "accidental" packages in isar-apt. I've implemented a very rough PoC, just to give you an idea. Please don't look too closely at the code, there are many things missing (e.g. SDKs, architectures != amd64, ...). But it can build isar-image-base for amd64. If we decide that this would be a worthwile change, I will spend some more time on it. Best, Adriaan Signed-off-by: Adriaan Schmidt <adriaan.schmidt@siemens.com> --- meta/classes/dpkg-base.bbclass | 46 ++++++++++--------- meta/classes/image-tools-extension.bbclass | 2 +- meta/classes/rootfs.bbclass | 27 ++++++----- meta/conf/bitbake.conf | 3 +- .../isar-bootstrap/isar-bootstrap.inc | 2 +- .../isar-apt/files/distributions.in | 3 -- meta/recipes-devtools/isar-apt/isar-apt.bb | 23 ---------- 7 files changed, 42 insertions(+), 64 deletions(-) delete mode 100644 meta/recipes-devtools/isar-apt/files/distributions.in delete mode 100644 meta/recipes-devtools/isar-apt/isar-apt.bb