From mboxrd@z Thu Jan 1 00:00:00 1970 X-GM-THRID: 6458972999552860160 X-Received: by 10.46.22.93 with SMTP id 29mr81525ljw.15.1504083289772; Wed, 30 Aug 2017 01:54:49 -0700 (PDT) X-BeenThere: isar-users@googlegroups.com Received: by 10.46.75.9 with SMTP id y9ls370778lja.17.gmail; Wed, 30 Aug 2017 01:54:49 -0700 (PDT) X-Google-Smtp-Source: ADKCNb6lRuilDPM5xwrA5ChJXA0MUES85v2EB5v1cccCNOIQFqlBZbgbVkzkRGtqKx6GbMYFdEzO X-Received: by 10.25.158.132 with SMTP id h126mr80411lfe.6.1504083289175; Wed, 30 Aug 2017 01:54:49 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1504083289; cv=none; d=google.com; s=arc-20160816; b=VCI/725f7KX7336ovI9LHn28EDtYB9zY5DVtkYD7vDQcC+pMLuJ+1eRoqd6JlFqYFY VNI/xUBKTyn/VtIyZ0Ofu+XXimmUfGdSZSW2MmLz5h9xvw1EqFIEvS38ZcCH19dixaoZ EUFE/ebjN8dnYrNla36OWSUkwzKABEcRptBa2K8dGg47Sj2ptxx7lgdff1TbwF8hEI6g 5QAN1FcfChXNzCy80FiYMrzhFWBNU2Dohvi7bSdz4XDDZ0rQhi88XN1+rwBl4N45MC8S EYt0lbegm961JtDGFqyzfDc96/rApVqF85dxXS76bEDJs76cdPuXPjfp+82D8XJrJYYr gqIQ== 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:to:subject :arc-authentication-results; bh=JMSPhnRdXxiwp0L/Xb4dbjeBbsBsTcCdlvYtaho1ccg=; b=nHC+Dk3/M2Cbl0qPHBRAn9KK6NSNqQ7d52eYl9WBiVFyiKcSv5cbOPuzCMqNZh7t8Y nKNFwFAqK8jgnbb9YUcQ1GeYnJIzsUqxTeLjleyJjJR0mmhJNexMQ4xFk0girIziIQL2 CmvaqLRrKqJGqjk8HtF8U1et0Szi8me04SCEOcmpLa10v+6QYZQmJldmgrTF56TGFgmz DHGXPEGFozEEd6R2+5F5vYs3f1YRx5NEFnGhW8KcZVrPLJ1zdyCHpC0A6grAPf51s0iZ E7eTSUw6Ix+JVOJoesDUj2NrJqjF5cOV3JqV6ZABacR1Oqbtvp9q7zZxLMmv2yqbJIjt 2O3w== ARC-Authentication-Results: i=1; gmr-mx.google.com; spf=neutral (google.com: 192.35.17.2 is neither permitted nor denied by best guess record for domain of claudius.heine.ext@siemens.com) smtp.mailfrom=claudius.heine.ext@siemens.com Return-Path: Received: from thoth.sbs.de (thoth.sbs.de. [192.35.17.2]) by gmr-mx.google.com with ESMTPS id w5si1549wmw.0.2017.08.30.01.54.49 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 30 Aug 2017 01:54:49 -0700 (PDT) Received-SPF: neutral (google.com: 192.35.17.2 is neither permitted nor denied by best guess record for domain of claudius.heine.ext@siemens.com) client-ip=192.35.17.2; Authentication-Results: gmr-mx.google.com; spf=neutral (google.com: 192.35.17.2 is neither permitted nor denied by best guess record for domain of claudius.heine.ext@siemens.com) smtp.mailfrom=claudius.heine.ext@siemens.com Received: from mail1.sbs.de (mail1.sbs.de [192.129.41.35]) by thoth.sbs.de (8.15.2/8.15.2) with ESMTPS id v7U8smHs023607 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Wed, 30 Aug 2017 10:54:48 +0200 Received: from [139.25.68.223] (linux-ses-ext02.ppmd.siemens.net [139.25.68.223]) by mail1.sbs.de (8.15.2/8.15.2) with ESMTP id v7U8sm1J014960; Wed, 30 Aug 2017 10:54:48 +0200 Subject: Re: [PATCH 3/6] classes/dpkg: Split install for cache To: Alexander Smirnov , isar-users@googlegroups.com References: <20170827151339.12806-1-asmirnov@ilbers.de> <20170827151339.12806-4-asmirnov@ilbers.de> <8e793c26-12cc-636a-6ea6-9cfadccf5f09@siemens.com> From: Claudius Heine Message-ID: <60d3f1ca-67a1-9c43-07f6-d2d89e4c2e51@siemens.com> Date: Wed, 30 Aug 2017 10:54:48 +0200 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: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-TUID: S66t7UY4Tn7n Hi, On 08/29/2017 09:18 AM, Alexander Smirnov wrote: > > > On 08/28/2017 11:00 AM, Claudius Heine wrote: >> Hi, >> >> On 08/27/2017 05:13 PM, Alexander Smirnov wrote: >>> Split install function into two parts: >>> - install_to_cache: if caching is enabled >>> - install_to_deploy: if caching is disabled >>> >>> This patch brings flexibility to the implementation and makes it >>> possible >>> to move all the caching implementation code to dedicated class. The >>> magic >>> behavior depending on the value of DEBCACHE_ENABLED is now >>> transparent in >>> the bitbake pipeline (can be inspected via "bitbake -g"). >>> >>> Signed-off-by: Alexander Smirnov >>> --- >>> meta/classes/dpkg.bbclass | 66 >>> ++++++++++++++++++++++++++-------------------- >>> meta/classes/image.bbclass | 2 +- >>> 2 files changed, 38 insertions(+), 30 deletions(-) >>> >>> diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass >>> index b1e201d..118ba2f 100644 >>> --- a/meta/classes/dpkg.bbclass >>> +++ b/meta/classes/dpkg.bbclass >>> @@ -56,46 +56,53 @@ do_build() { >>> } >>> # Install package to dedicated deploy directory >>> -do_binary_deb_install() { >>> +do_install_to_deploy() { >>> + # Deb caching is disabled, simply copy all binary packages to >>> the deploy >>> + # directory >>> + install -m 644 "${BUILDROOT}"/*.deb "${DEPLOY_DIR_DEB}/" >>> +} >>> + >>> +addtask install_to_deploy after do_build >>> +do_install_to_deploy[dirs] = "${DEPLOY_DIR_DEB}" >>> +do_install_to_deploy[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}" >>> +do_install_to_deploy[noexec] = "1" >>> + >>> +# Install package to dedicated apt repo >>> +do_install_to_cache() { >>> 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 >>> - >>> - # 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}/" >>> + # 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 >>> + >>> + # 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 >>> } >>> -addtask binary_deb_install after do_build >>> -do_binary_deb_install[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}" >>> +addtask install_to_cache after do_build >> >> What was the reason why we put stuff after the 'build' task? In OE the >> 'build' task is the default task triggering everything necessary for a >> recipe. In Isar that changed. Why? > > Actually AFAIK do_build is not OE tasks, it's default bitbake task. > Install is after do_build, because in Isar do_build is used to build the > packages, so it's not empty. You are right, its a bitbake task. Since I have much more experience with OE than with just bitbake or isar, I referenced my question to the experience I had using OE. I cannot remember seeing tasks that are added after the do_build task in OE and I still find that very strange. AFAIK the 'build' task in bitbake and OE has a similar meaning to the 'all' target in make files. The idea to add statements to the 'all' target or letting other targets depend on the 'all' target sounds very odd to me. And that is what is done here in isar. Just very strange and unusual for my eyes and I don't see the advantage of this. I would like to hear the reason for this decision. Why have you added statements to the 'build' task and why are other tasks depend on it? >> >>> +do_install_to_cache[stamp-extra-info] = "${DISTRO}-${DISTRO_ARCH}" >>> +do_install_to_cache[noexec] = "1" >>> # 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 >>> + # Deb caching is disabled >>> + d.delVarFlag('do_install_to_deploy', 'noexec') >>> return True >>> PN = d.getVar("PN", True) >>> @@ -108,6 +115,7 @@ python __anonymous () { >>> path_cache = os.path.join(DEBCACHEDIR, DISTRO) >>> path_databases = os.path.join(DEBDBDIR, DISTRO) >>> path_distributions = os.path.join(path_cache, "conf", >>> "distributions") >>> + d.delVarFlag('do_install_to_cache', 'noexec') >>> # The distributions file is needed by `reprepro` to know what >>> types >>> # of packages are supported, what the distribution name is, etc. >>> diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass >>> index c2ff453..6b1b5eb 100644 >>> --- a/meta/classes/image.bbclass >>> +++ b/meta/classes/image.bbclass >>> @@ -46,4 +46,4 @@ do_populate() { >>> } >>> addtask populate before do_build >>> -do_populate[deptask] = "do_install" >>> +do_populate[deptask] = "do_install_to_cache do_install_to_deploy" >> >> I would rather still have a, maybe empty, 'do_install' step that just >> depends on the 'do_install_to_cache' and 'do_install_to_deploy' tasks. >> I don't see why ever small change in one class should change the >> complete pipeline for every recipe. > > It's a bit difficult question. In general meaning when building > something from sources, the install means a storing of build artifacts > to some specific location. So in this case, we just copy binary deb > package to repo, and there is nothing common with installation of > artifacts. > > Also Isar has in roadmap cross-compilation, so the pipeline will > probably look like: > > fetch - unpack - compile - install - install_to_cache > > so in this case the install task probably should go before > install_to_cache. > > So probably it makes sense to avoid _install_ in this task, for example > use do_populate_deb_cache, what do you think? Something like: 'populate_repo' sounds much better for this case. I would try to have more abstract names at first and implement them with more specific names like 'populate_repo_deb'. In OE there already is a 'package' and additional sub tasks like 'package_write_deb'. So a general pipeline for normal recipes like this: fetch, unpack, configure, compile, install, package, populate_repo Of course image recipes would have a slightly changed pipeline. I think the discussion about the build pipeline in isar is very important. Maybe we should make an extra thread about this. Cheers, Claudius -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-54 Fax: (+49)-8142-66989-80 Email: ch@denx.de