From mboxrd@z Thu Jan 1 00:00:00 1970 X-GM-THRID: 6458972999552860160 X-Received: by 10.28.142.74 with SMTP id q71mr77934wmd.17.1503988855176; Mon, 28 Aug 2017 23:40:55 -0700 (PDT) X-BeenThere: isar-users@googlegroups.com Received: by 10.28.215.211 with SMTP id o202ls132000wmg.22.gmail; Mon, 28 Aug 2017 23:40:54 -0700 (PDT) X-Google-Smtp-Source: ADKCNb4ruby4VkO+dXLFbAoWqeHOw8JkVmSbV+jVe3HX1QF29uilasqdx8t8HwnuIVOtNY5cqybp X-Received: by 10.28.139.77 with SMTP id n74mr235333wmd.11.1503988854773; Mon, 28 Aug 2017 23:40:54 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1503988854; cv=none; d=google.com; s=arc-20160816; b=phic7F2VX6ZFFjwUex+EbCTf81XOXTHy41q/dPSoNXyRfGk4pnu4u7BKsHiH5H7eFy MNSn2NRv7jst345d0ldgoJ3M53PYn328vXpauCt1xwqIaMOlM/+4AxR79bNlosW2PwT7 QoZMjtkyvSk/yKu7F0hd3rQBD9QvKBZLUtEf08+M67PoSQtiVYpcrL0Vkin1bKqBC74H 1U2AeOOKO+8dcm6aWBcBJGaNDZCk1RyTO9lOSCa/aiA2utmYEd/eCPKst63denCUElzW 5qXgSfU/ZS+Ree4zCJQJhEYzN+RBl1GmxEGjTfglsgxcxSO4Kj4Q89YL+oe5MSkiy2Hg TG3w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:cc:to:subject :arc-authentication-results; bh=jhKHGYRwVtaHasd3j+ttIRQxh00U0qk4ce551Yn61RY=; b=saHK5emx33YlxDkvL8kCHlqy1r1W4TDZByS7yMpkQlR2WFRxZRJzMBfBafTrZQfMOu c+1c71vcN2WUGEQiFmlp28n5J4/vBl8HHRuTPUiZAnwgQ9SbeTjWdKBBv7m/2lN+T4PY inihNbkCbsUITxFX1GScr163fxDZ5ECce/Sqg2zHvsiZclnEukfYRqsQSpJVzK3oftNq 3ikVzMJeH2gdHwPzKVzoBpwmUVuGVabmSB5NrNRk2kXv2COhTCFWc/9W6TfnpnGTJZNS E/exD55etgM/RiGyqnLr3NsRWLRL1L2FAQJcXTEJSdij/kRUdF5771ZKS5NXyJg7gB8l zRLQ== ARC-Authentication-Results: i=1; gmr-mx.google.com; spf=pass (google.com: best guess record for domain of asmirnov@ilbers.de designates 85.214.62.211 as permitted sender) smtp.mailfrom=asmirnov@ilbers.de Return-Path: Received: from aqmola.ilbers.de (aqmola.ilbers.de. [85.214.62.211]) by gmr-mx.google.com with ESMTPS id m11si140801wmi.9.2017.08.28.23.40.54 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 28 Aug 2017 23:40:54 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of asmirnov@ilbers.de designates 85.214.62.211 as permitted sender) client-ip=85.214.62.211; Authentication-Results: gmr-mx.google.com; spf=pass (google.com: best guess record for domain of asmirnov@ilbers.de designates 85.214.62.211 as permitted sender) smtp.mailfrom=asmirnov@ilbers.de Received: from [10.0.2.15] ([188.227.110.165]) (authenticated bits=0) by aqmola.ilbers.de (8.14.4/8.14.4/Debian-4+deb7u1) with ESMTP id v7T6epCm013158 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); Tue, 29 Aug 2017 08:40:53 +0200 Subject: Re: [PATCH 1/6] meta-isar-bin: Enable caching of deb packages To: Henning Schild Cc: isar-users@googlegroups.com References: <20170827151339.12806-1-asmirnov@ilbers.de> <20170827151339.12806-2-asmirnov@ilbers.de> <20170828171842.34b51b95@md1em3qc> From: Alexander Smirnov Message-ID: Date: Tue, 29 Aug 2017 09:40:46 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20170828171842.34b51b95@md1em3qc> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-TUID: FjEW17uqBmnl On 08/28/2017 06:18 PM, Henning Schild wrote: > This patch is very big, doing a lot of things at a time. Ok, I'll think how it can be split. > The commit message on the other hand is not very verbose. Half of the patch is the comments that describe in details what happens in the code, so please questions and proposals. > > creating the repo is one step > - i think using reprepro is a good idea > moving do_populate to that repo is another one > - i think this task should go away and the repo should be given to > multistrap Ok > and there are probably more ways to split that into reviewable pieces Please questions and proposals. > > So this review is not too verbose, we need smaller pieces. > > More inline > > Am Sun, 27 Aug 2017 18:13:34 +0300 > schrieb Alexander Smirnov : > >> From: Frank Lenormand >> >> Adds the possibility to create an apt repository for newly built >> packages and then use that repo for populating the target filesystem. >> >> Signed-off-by: Frank Lenormand >> Signed-off-by: Alexander Smirnov >> --- >> meta-isar-bin/conf/layer.conf | 18 +++ >> meta-isar-bin/files/distributions.in | 3 + >> meta-isar/conf/bblayers.conf.sample | 1 + >> .../images/files/debian-configscript.sh | 8 ++ >> .../images/files/raspbian-configscript.sh | 8 ++ >> meta-isar/recipes-core/images/isar-image-base.bb | 8 +- >> meta/classes/dpkg.bbclass | 126 >> +++++++++++++++++++-- >> meta/classes/image.bbclass | 30 +++-- 8 >> files changed, 186 insertions(+), 16 deletions(-) create mode 100644 >> meta-isar-bin/conf/layer.conf create mode 100644 >> meta-isar-bin/files/distributions.in >> >> diff --git a/meta-isar-bin/conf/layer.conf >> b/meta-isar-bin/conf/layer.conf new file mode 100644 >> index 0000000..24534e1 >> --- /dev/null >> +++ b/meta-isar-bin/conf/layer.conf >> @@ -0,0 +1,18 @@ >> +# Enable package caching with '1' >> +DEBCACHE_ENABLED ?= "0" >> + >> +# Codename of the repository created by the caching class >> +DEBDISTRONAME = "isar" >> + >> +# Path to the caching repository >> +DEBCACHEDIR ?= "${LAYERDIR}/apt" >> + >> +# Path to the mount point of the repository within the target >> rootfs, during +# population >> +DEBCACHEMNT ?= "/opt/cache/apt" >> + >> +# Path to the databases used by `reprepro` >> +DEBDBDIR ?= "${LAYERDIR}/db" >> + >> +# Path to the configuration files templates used by `reprepro` >> +DEBFILESDIR ?= "${LAYERDIR}/files" >> diff --git a/meta-isar-bin/files/distributions.in >> b/meta-isar-bin/files/distributions.in new file mode 100644 >> index 0000000..59f4429 >> --- /dev/null >> +++ b/meta-isar-bin/files/distributions.in >> @@ -0,0 +1,3 @@ >> +Codename: {DISTRO_NAME} >> +Architectures: i386 armhf source >> +Components: main >> diff --git a/meta-isar/conf/bblayers.conf.sample >> b/meta-isar/conf/bblayers.conf.sample index 80867e7..53a362b 100644 >> --- a/meta-isar/conf/bblayers.conf.sample >> +++ b/meta-isar/conf/bblayers.conf.sample >> @@ -8,6 +8,7 @@ BBFILES ?= "" >> BBLAYERS ?= " \ >> ##ISARROOT##/meta \ >> ##ISARROOT##/meta-isar \ >> + ##ISARROOT##/meta-isar-bin \ >> " >> BBLAYERS_NON_REMOVABLE ?= " \ >> ##ISARROOT##/meta \ >> diff --git >> a/meta-isar/recipes-core/images/files/debian-configscript.sh >> b/meta-isar/recipes-core/images/files/debian-configscript.sh index >> 4ac37d0..b05babb 100644 --- >> a/meta-isar/recipes-core/images/files/debian-configscript.sh +++ >> b/meta-isar/recipes-core/images/files/debian-configscript.sh @@ -8,6 >> +8,8 @@ set -e readonly MACHINE_SERIAL="$1" readonly BAUDRATE_TTY="$2" >> readonly ROOTFS_DEV="$3" >> +readonly DEBCACHEMNT="$4" >> +readonly DEBDISTRONAME="$5" >> >> cat >> /etc/default/locale << EOF >> LANG=en_US.UTF-8 >> @@ -83,3 +85,9 @@ fi >> if [ -x "$TARGET/sbin/init" -a -x "$TARGET/usr/sbin/policy-rc.d" ]; >> then rm -f $TARGET/usr/sbin/policy-rc.d >> fi >> + >> +mkdir -p /etc/apt/sources.list.d/ >> +cat </etc/apt/sources.list.d/${DEBDISTRONAME}.list >> +deb file:${DEBCACHEMNT}/ ${DEBDISTRONAME} main >> +deb-src file:${DEBCACHEMNT}/ ${DEBDISTRONAME} main >> +EOF >> diff --git >> a/meta-isar/recipes-core/images/files/raspbian-configscript.sh >> b/meta-isar/recipes-core/images/files/raspbian-configscript.sh index >> 2454481..f995f4d 100644 --- >> a/meta-isar/recipes-core/images/files/raspbian-configscript.sh +++ >> b/meta-isar/recipes-core/images/files/raspbian-configscript.sh @@ >> -8,6 +8,8 @@ set -e readonly MACHINE_SERIAL="$1" readonly >> BAUDRATE_TTY="$2" readonly ROOTFS_DEV="$3" >> +readonly DEBCACHEMNT="$4" >> +readonly DEBDISTRONAME="$5" >> >> cat >> /etc/default/locale << EOF >> LANG=en_US.UTF-8 >> @@ -89,3 +91,9 @@ KERNEL_IMAGE=`ls /boot | grep vmlinuz` >> cat > /boot/config.txt << EOF >> kernel=$KERNEL_IMAGE >> EOF >> + >> +mkdir -p /etc/apt/sources.list.d/ >> +cat </etc/apt/sources.list.d/${DEBDISTRONAME}.list >> +deb file:${DEBCACHEMNT}/ ${DEBDISTRONAME} main >> +deb-src file:${DEBCACHEMNT}/ ${DEBDISTRONAME} main >> +EOF >> diff --git a/meta-isar/recipes-core/images/isar-image-base.bb >> b/meta-isar/recipes-core/images/isar-image-base.bb index >> b679d97..85e6b27 100644 --- >> a/meta-isar/recipes-core/images/isar-image-base.bb +++ >> b/meta-isar/recipes-core/images/isar-image-base.bb @@ -49,8 +49,12 @@ >> do_rootfs() { sudo multistrap -a ${DISTRO_ARCH} -d "${S}" -f >> "${WORKDIR}/multistrap.conf" || true >> # Configure root filesystem >> - sudo chroot ${S} /configscript.sh ${MACHINE_SERIAL} >> ${BAUDRATE_TTY} \ >> - ${ROOTFS_DEV} >> + sudo chroot ${S} /configscript.sh \ >> + ${MACHINE_SERIAL} \ >> + ${BAUDRATE_TTY} \ >> + ${ROOTFS_DEV} \ >> + ${DEBCACHEMNT} \ >> + ${DEBDISTRONAME} >> sudo rm ${S}/configscript.sh >> } >> >> diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass >> index 360a95c..b1e201d 100644 >> --- a/meta/classes/dpkg.bbclass >> +++ b/meta/classes/dpkg.bbclass >> @@ -1,5 +1,5 @@ >> # This software is a part of ISAR. >> -# Copyright (C) 2015-2016 ilbers GmbH >> +# Copyright (C) 2015-2017 ilbers GmbH >> >> # Add dependency from buildchroot creation >> DEPENDS += "buildchroot" >> @@ -55,12 +55,124 @@ do_build() { >> sudo chroot ${BUILDCHROOT_DIR} /build.sh ${PP}/${SRC_DIR} >> } >> >> - >> # Install package to dedicated deploy directory >> -do_install() { >> - install -m 644 ${BUILDROOT}/*.deb ${DEPLOY_DIR_DEB}/ >> +do_binary_deb_install() { >> + readonly DIR_CACHE="${DEBCACHEDIR}/${DISTRO}" >> + readonly DIR_DB="${DEBDBDIR}/${DISTRO}" >> + >> + if [ "${DEBCACHE_ENABLED}" != "0" ]; then >> + # If `bitbake` is running for the first time, the cache >> doesn't exist >> + # yet and needs to be configured using a `distributions` >> file. >> + # A template stored in the layer directory is pre-processed >> to >> + # generate the configuration file, which is then placed in >> the >> + # appropriate directory. >> + if [ ! -e "${DIR_CACHE}/conf/distributions" ]; then >> + mkdir -p "${DIR_CACHE}/conf" >> + sed -e "s#{DISTRO_NAME}#${DEBDISTRONAME}#g" \ >> + "${DEBFILESDIR}/distributions.in" \ >> + > "${DIR_CACHE}/conf/distributions" >> + fi > > This step should be in the image and not in every package. I guess it > has a race with multiple packages running this init routine. Ok. > >> + # Add binary and source packages to the deb cache >> + # If the cache doesn't exist yet, it will be created using >> the >> + # `distributions` file generated above. >> + ls -1 "${BUILDROOT}"/*.deb "${BUILDROOT}"/*.dsc | while read >> -r p; do >> + reprepro --waitforlock 3 -b "${DIR_CACHE}" --dbdir >> "${DIR_DB}" \ >> + -C main "include${p##*.}" "${DEBDISTRONAME}" "${p}" >> + done >> + else >> + # Deb caching is disabled, simply copy all binary packages >> to the >> + # deploy directory >> + mkdir -p "${DEPLOY_DIR_DEB}" >> + install -m 644 "${BUILDROOT}"/*.deb "${DEPLOY_DIR_DEB}/" >> + fi >> } >> >> -addtask install after do_build >> -do_install[dirs] = "${DEPLOY_DIR_DEB}" >> -do_install[stamp-extra-info] = "${MACHINE}" >> +addtask binary_deb_install after do_build >> +do_binary_deb_install[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}" > > Here a task got renamed. This should be another patch ... one that i > already tried to get in. > This implemented in the next patch (to keep this patch in original format), but yes, it should be done here. >> + >> +# Deb caching lambda run during the parsing phase that checks >> whether the +# current package has to be rebuilt, or taken from the >> cache +python __anonymous () { >> + if d.getVar("DEBCACHE_ENABLED", True) == "0": >> + # Deb caching is disabled, do nothing >> + return True >> + >> + PN = d.getVar("PN", True) >> + PV = d.getVar("PV", True) >> + DISTRO_ARCH = d.getVar("DISTRO_ARCH", True) >> + DEBCACHEDIR = d.getVar("DEBCACHEDIR", True) >> + DEBDISTRONAME = d.getVar("DEBDISTRONAME", True) >> + DEBDBDIR = d.getVar("DEBDBDIR", True) >> + DISTRO = d.getVar("DISTRO", True) >> + path_cache = os.path.join(DEBCACHEDIR, DISTRO) >> + path_databases = os.path.join(DEBDBDIR, DISTRO) >> + path_distributions = os.path.join(path_cache, "conf", >> "distributions") + >> + # The distributions file is needed by `reprepro` to know what >> types >> + # of packages are supported, what the distribution name is, etc. >> + # If it doesn't exist, we have nothing in the cache, do nothing. >> + if not os.path.exists(path_distributions): >> + return >> + >> + # Anonymous functions are run several times under different >> contexts >> + # during the parsing phase, which would let the code that >> follows be run >> + # as many times for the same package. >> + # In order to guarantee that our subroutine only runs once per >> package, we >> + # use bitbake's "persist" API in order to have reliable >> persistent storage >> + # accross calls of the lambda (using a simple variable in the >> class won't >> + # work, as several contexts won't allow fetching its value). >> + pd = bb.persist_data.persist("DEBCACHE_PACKAGES", d) >> + if PN in pd and pd[PN] == PV: >> + return >> + >> + import subprocess >> + try: >> + # The databases used by `reprepro` are not stored within the >> cache in >> + # order to make versioning of only the files needed to use >> the cache >> + # as an official Debian repository simpler. >> + # As such, if a developer uses a peer's cache to speed up >> their build >> + # time but have never run bitbake, the database will not >> have been >> + # created, so we regenerate them here. >> + if not os.path.exists(path_databases) and >> os.path.exists(path_cache): >> + bb.note("Regenerating the cache databases...") >> + subprocess.check_call([ >> + "reprepro", >> + "--waitforlock", "3", >> + "-b", path_cache, >> + "--dbdir", path_databases, >> + "export", DEBDISTRONAME, >> + ]) >> + >> + # Get a list of the versions of all the packages named after >> the >> + # current bitbake package, and check whether the current >> package >> + # version is returned. >> + # As `reprepro` always returns zero with this particular >> operation, we >> + # have to use this workaround to check for a package in the >> cache. >> + package_version = subprocess.check_output([ >> + "reprepro", >> + "--waitforlock", "3", >> + "-b", path_cache, >> + "--dbdir", path_databases, >> + "-C", "main", >> + "-A", DISTRO_ARCH, >> + "--list-format", "${version}", >> + "list", DEBDISTRONAME, PN, >> + ]) >> + package_version = package_version.decode("utf-8") >> + if package_version == PV: >> + # The below list contains the names of all the tasks are >> in charge >> + # of building the package when the cache isn't enabled >> or if the >> + # package hasn't been placed in it already. >> + # As all tasks are enabled by default, we prevent their >> execution >> + # by setting the `noexec` flag, which will prevent a >> rebuild of >> + # the package when it's cached. >> + for task in ["fetch", "unpack", "build", "install"]: >> + d.setVarFlag("do_{}".format(task), "noexec", "1") >> + >> + # Cache the results of this command so that subsequent >> executions >> + # of this anonymous functions don't run the same code >> again. >> + pd[PN] = PV >> + except subprocess.CalledProcessError as e: >> + bb.fatal("Unable to check for a candidate for package {0} >> (errorcode: {1})".format(PN, e.returncode)) +} >> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass >> index a7f0d74..f42aa48 100644 >> --- a/meta/classes/image.bbclass >> +++ b/meta/classes/image.bbclass >> @@ -11,18 +11,34 @@ inherit ${IMAGE_TYPE} >> >> do_populate[stamp-extra-info] = "${MACHINE}-${DISTRO}" >> >> -# Install Debian packages, that were built from sources >> +# Install Debian packages from the cache >> do_populate() { >> + readonly DIR_CACHE="${DEBCACHEDIR}/${DISTRO}" >> + >> if [ -n "${IMAGE_INSTALL}" ]; then >> - sudo mkdir -p ${S}/deb >> + if [ "${DEBCACHE_ENABLED}" != "0" ]; then >> + sudo mkdir -p "${S}/${DEBCACHEMNT}" >> + sudo mount -o bind "${DIR_CACHE}" "${S}/${DEBCACHEMNT}" >> + >> + sudo chroot "${S}" apt-get update -y > > This will fail if you are behind a proxy. It makes a known issue worse. > >> + for package in ${IMAGE_INSTALL}; do >> + sudo chroot "${S}" apt-get install -t >> "${DEBDISTRONAME}" -y \ >> + --allow-unauthenticated "${package}" >> + done >> + >> + sudo umount "${S}/${DEBCACHEMNT}" > > A whole lot of sudo. I do not mind but am still wondering ... Please questions and proposals instead of negative tone. > > Can we replace the bind-mount with a symlink? If anything between the > mount and umount fails and you need to run again ... will the mount > fail if the old one is still there? If we switch to multistrap-based populating, I think this will go. > >> + else >> + sudo mkdir -p ${S}/deb >> >> - for p in ${IMAGE_INSTALL}; do >> - sudo cp ${DEPLOY_DIR_DEB}/${p}_*.deb ${S}/deb >> - done >> + for p in ${IMAGE_INSTALL}; do >> + find "${DEPLOY_DIR_DEB}" -type f -name '*.deb' -exec >> \ >> + sudo cp '{}' "${S}/deb/" \; >> + done >> >> - sudo chroot ${S} /usr/bin/dpkg -i -R /deb >> + sudo chroot ${S} /usr/bin/dpkg -i -R /deb >> >> - sudo rm -rf ${S}/deb >> + sudo rm -rf ${S}/deb >> + fi >> fi >> } >> >