From mboxrd@z Thu Jan 1 00:00:00 1970 X-GM-THRID: 6816610675337461760 X-Received: by 2002:a1c:4e0a:: with SMTP id g10mr4015448wmh.71.1599159504913; Thu, 03 Sep 2020 11:58:24 -0700 (PDT) X-BeenThere: isar-users@googlegroups.com Received: by 2002:a05:6000:8:: with SMTP id h8ls7956986wrx.3.gmail; Thu, 03 Sep 2020 11:58:24 -0700 (PDT) X-Google-Smtp-Source: ABdhPJw491CAnSmvxBVHuawOKLCR1kmcQLchF6QoHngw5Yrv02diai/BB3LrN9ZzqjdXlF4JQDUB X-Received: by 2002:a5d:620e:: with SMTP id y14mr4178966wru.371.1599159504266; Thu, 03 Sep 2020 11:58:24 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1599159504; cv=none; d=google.com; s=arc-20160816; b=0zJgh3pJgwnTQDwaPDekpuWxu3FO0HPwL9jwsmVLrF5ModZnGi6ZvQhzgCe/9BVzr2 jVt/Ysb7H1EQkzjOI00AFHMjYTA7kGJ5zPehspsLJn6PKZsT46zPZVtbhhHbryzlhLW0 CG5o2Rv+8DpqI2TV8C6NKjy/r8XtfgSavFJNlTnZDNzlkmYgJxR6p7ZlrZsGhnDNPWQE A4Q4grscilmYV9t3RGq2X1BlmiPmsA3Ugwh2ehZ8tVl6b2rvQmTPdYQXkYcXlm6Jp29J /fUmPyHtMt5ULkNiIBq5Fs9NKAum73wzcGTDH3xjBnbjJfVhTZ5x/uTs/gh2Doo7NAth +VfQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date; bh=X29NaHu1KIJSCAjqC0sQFkyLQ/3hP7EqCgsh/96jYj4=; b=ja2TlYX7N62YwOXsJIag+MDBg/Gk9I4MZ6uY8xnhYEROgXW+tS6KVg+FxXaF6Qu2Wj y4hSghfOvs261B6seT7nubak3QpV/2Jkg5AN3/+cfd2FPuW/jNWJOts3CGRvQdjhj3Mz 8fYsAfHwpAh4kjfUpav02AZt7H3q4Ts/TkesjJvlIHHTTipDVi9vvUnPiAhyGxJkyvPt KmMFkvfNx1BcrEkhtgr6+vVHipBM0Rc+0s8/wBEKBkuCQUE+eQeLlEuVOawPRcxNpPil lOlPtLrg0f9cuWNR+1GpJ0TENajGLErA+eVPkVSP77DmJdQWcICj1ZsqY1zsEfkkX7tb 6Wow== ARC-Authentication-Results: i=1; gmr-mx.google.com; spf=pass (google.com: domain of henning.schild@siemens.com designates 192.35.17.28 as permitted sender) smtp.mailfrom=henning.schild@siemens.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=siemens.com Return-Path: Received: from goliath.siemens.de (goliath.siemens.de. [192.35.17.28]) by gmr-mx.google.com with ESMTPS id m3si145575wme.0.2020.09.03.11.58.24 for (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 03 Sep 2020 11:58:24 -0700 (PDT) Received-SPF: pass (google.com: domain of henning.schild@siemens.com designates 192.35.17.28 as permitted sender) client-ip=192.35.17.28; Authentication-Results: gmr-mx.google.com; spf=pass (google.com: domain of henning.schild@siemens.com designates 192.35.17.28 as permitted sender) smtp.mailfrom=henning.schild@siemens.com; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=siemens.com Received: from mail2.sbs.de (mail2.sbs.de [192.129.41.66]) by goliath.siemens.de (8.15.2/8.15.2) with ESMTPS id 083IwNPD024990 (version=TLSv1.2 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 3 Sep 2020 20:58:23 +0200 Received: from md1za8fc.ad001.siemens.net ([167.87.32.163]) by mail2.sbs.de (8.15.2/8.15.2) with ESMTP id 083IwL5o024113; Thu, 3 Sep 2020 20:58:22 +0200 Date: Thu, 3 Sep 2020 20:58:21 +0200 From: Henning Schild To: "vijaikumar....@gmail.com" Cc: isar-users Subject: Re: [PATCH v5 03/13] meta: cache deb srcs as part of postprocessing Message-ID: <20200903205821.4affc9f5@md1za8fc.ad001.siemens.net> In-Reply-To: References: <20200417093040.15130-1-Vijaikumar_Kanagarajan@mentor.com> <20200417093040.15130-4-Vijaikumar_Kanagarajan@mentor.com> <20200422090639.645bf93b@md1za8fc.ad001.siemens.net> <20200429081937.7e6a059d@md1za8fc.ad001.siemens.net> <2043181c-a74f-4bfa-a8dd-36916d87c962@googlegroups.com> X-Mailer: Claws Mail 3.17.6 (GTK+ 2.24.32; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-TUID: GZVzOGSH1IKE Hi Vijay, sorry i never really finished the review on this one. I hope the maintainer will take action and look into it. I would be happy to look at parts again but currently do not have the time to review further. Henning On Thu, 3 Sep 2020 04:43:13 -0700 (PDT) "vijaikumar....@gmail.com" wrote: > Ping > > On Thursday, June 11, 2020 at 5:14:38 PM UTC+5:30 > vijaikumar....@gmail.com wrote: > > > Hi Henning, > > > > Did you get a chance to review the rest of the series? > > > > Thanks, > > Vijai Kumar K > > > > On Fri, May 8, 2020 at 11:14 AM vijai kumar > > wrote: > > > > > > > > > > > > On Monday, May 4, 2020 at 2:12:38 PM UTC+5:30, vijai kumar wrote: > > > > > >> > > >> > > >> > > >> On Wednesday, April 29, 2020 at 11:49:41 AM UTC+5:30, Henning > > >> Schild > > wrote: > > >>> > > >>> Am Wed, 22 Apr 2020 02:57:13 -0700 (PDT) > > >>> schrieb vijai kumar : > > >>> > > >>> > On Wednesday, April 22, 2020 at 12:36:42 PM UTC+5:30, Henning > > >>> > Schild wrote: > > >>> > > > > >>> > > On Fri, 17 Apr 2020 15:00:30 +0530 > > >>> > > Vijai Kumar K > wrote: > > >>> > > > > >>> > > > Collect the deb sources of the corresponding deb binaries > > >>> > > > cached in DEBDIR as part of image postprocess. > > >>> > > > > > >>> > > > Signed-off-by: Vijai Kumar K > >>> > > > > --- > > >>> > > > meta/classes/deb-dl-dir.bbclass | 39 > > >>> > > > +++++++++++++++++++++++++++++++++ > > >>> > > > meta/classes/image.bbclass | 2 +- > > >>> > > > meta/classes/rootfs.bbclass | 8 +++++++ 3 files changed, > > >>> > > > 48 insertions(+), 1 deletion(-) > > >>> > > > > > >>> > > > diff --git a/meta/classes/deb-dl-dir.bbclass > > >>> > > > b/meta/classes/deb-dl-dir.bbclass index 29a3d67..472b9fe > > >>> > > > 100644 --- a/meta/classes/deb-dl-dir.bbclass > > >>> > > > +++ b/meta/classes/deb-dl-dir.bbclass > > >>> > > > @@ -5,6 +5,45 @@ > > >>> > > > > > >>> > > > inherit repository > > >>> > > > > > >>> > > > +debsrc_download() { > > >>> > > > + export rootfs="$1" > > >>> > > > + export rootfs_distro="$2" > > >>> > > > + mkdir -p "${DEBSRCDIR}"/"${rootfs_distro}" > > >>> > > > + ( flock 9 > > >>> > > > > >>> > > I think you can grab that lock only for the actual writes, > > >>> > > and keep the generation of the list out of the critical > > >>> > > section. > > >>> > > > >>> > To note, this lock also guards the mount part. > > >>> > > >>> Any why does that need to be under the lock? What is the > > >>> essence of the lock anyways? > > >> > > >> > > >> We are mounting DEBSRCDIR onto the image rootfs and then > > >> downloading the deb srcs on-to that. Once that is done, we are > > >> unmounting it. The > > lock > > >> makes sure that there is no race condition between mounts and > > >> unmounts. Not seen such races but there could be a situation > > >> where in the first > > builds unmount > > >> is called after the second builds mount check. > > > > > > > > > Looking back, I don't think there will ever be a case like this > > > for > > image rootfs. > > > The lock may not be needed for mounts. > > > > > >> > > >> On an alternate way, we could just mount DL_DIR in > > >> rootfs_do_mounts and > > take care > > >> of the cleanup in rootfs_finalize. That way we can avoid this > > additional mount. > > >> > > >> > > >>> As far as i understand there are multiple writers potentially > > >>> creating the same files in DEBSRCDIR. If that is a problem we > > >>> also need locking in do_apt_fetch. While one multiconfig image > > >>> is in your postprocess, another might still be fetching that > > >>> same sources. > > >> > > >> > > >> > > >> As I see, there are only two writers who write to DEBSRCDIR. > > >> 1. The post process caching function from this series. > > >> 2. Fetch case using SRC_URI=apt:// > > >> > > >> Most of the package sources are fetched via postprocess. And > > >> with lock > > in > > >> place no two deb-src caching takes place at the same time. > > >> > > >> For fetch case using SRC_URI=apt://, say Package X. > > >> > > >> Assume there are two multiconfig images A and B both include > > >> the recipe which provides Package X. In that case when image A > > >> is in > > postprocess > > >> deb-src caching, Package X source would already be available in > > >> > > DEBSRCDIR. > > >> If multiconfig image B is fetching package X when image A is in > > >> > > postprocess > > >> accessing it, there would be no issue, since apt-get source > > download-only does not > > >> re-download the package. > > >> > > >> > > >>> > > >>> > > >>> > > Note that i played with this "flock 9" syntax instead of > > >>> > > what is used in deb-dl-dir, it did not work as expected. > > >>> > > Probably because it will be many shells and 9 is a > > >>> > > different fd in all of them. > > >>> > > > >>> > Interesting. Works as expected here. If we still need to > > >>> > switch the syntax to be sure, we could. > > >>> > > >>> So you did try multiconfig and two or more writers never ran at > > >>> the same time? > > >> > > >> > > >> Yes. > > >> > > >>> > > >>> In deb-dl-dir there is exclusive writer locking and shared > > >>> reader locking, maybe that is why i decided against "flock 9". > > >>> > > >>> > > + set -e > > >>> > > > + printenv | grep -q BB_VERBOSE_LOGS && set -x > > >>> > > > + sudo -E -s <<'EOSUDO' > > >>> > > > + mkdir -p "${rootfs}/deb-src" > > >>> > > > + mountpoint -q "${rootfs}/deb-src" || \ > > >>> > > > + mount --bind "${DEBSRCDIR}" "${rootfs}/deb-src" > > >>> > > > +EOSUDO > > >>> > > > + find "${rootfs}/var/cache/apt/archives/" -maxdepth 1 > > >>> > > > -type f -iname '*\.deb' | while read package; do > > >>> > > > + local src="$( dpkg-deb --show --showformat '${Source}' > > >>> > > > "${package}" )" > > >>> > > > + # If the binary package version and source package > > >>> > > > version are different, then the > > >>> > > > + # source package version will be present inside "()" of > > >>> > > > the Source field. > > >>> > > > > >>> > > dpkg-query(1) > > >>> > > > > >>> > > dpkg-deb --show --showformat '${source:Version}' > > >>> > > dpkg-deb --show --showformat '${source:Upstream-Version}' > > >>> > > > > >>> > > might help to write this cleaner. > > >>> > > > > >>> > > > >>> > Thanks. Will use this. > > >>> > > > >>> > > > >>> > > > > >>> > > > + local version="$( echo "$src" | cut -sd "(" -f2 | cut > > >>> > > > -sd ")" -f1 )" > > >>> > > > + if [ -z ${version} ]; then > > >>> > > > + version="$( dpkg-deb --show --showformat > > >>> > > > '${Version}' "${package}" )" > > >>> > > > + fi > > >>> > > > + # Now strip any version information that might be > > >>> > > > available. > > >>> > > > + src="$( echo "$src" | cut -d' ' -f1 )" > > >>> > > > + # If there is no source field, then the source package > > >>> > > > has the same name as the > > >>> > > > + # binary package. > > >>> > > > + if [ -z "${src}" ];then > > >>> > > > + src="$( dpkg-deb --show --showformat '${Package}' > > >>> > > > "${package}" )" > > >>> > > > + fi > > >>> > > > > >>> > > I still could not find those proxies that all downloading > > >>> > > functions need in their env. > > >>> > > > > >>> > > > >>> > From what I see, the rootfs class from where this is called, > > >>> > already takes care of this > > >>> > > > >>> > > > https://github.com/ilbers/isar/blob/next/meta/classes/rootfs.bbclass#L22 > > > > >>> > > >>> I see. It might be a good idea to run a local test with > > >>> BB_NO_NETWORK. That will show whether the proxies really > > >>> "arrive" and you will learn how you feature and that switch > > >>> work together. > > >> > > >> > > >> I did an offline build(with BB_NO_NETWORK set) with this series > > >> and it > > seems > > >> to work fine. > > >> > > >> Thanks, > > >> Vijai Kumar K > > >> > > >>> > > >>> Henning > > >>> > > >>> > > > >>> > > > > >>> > > Henning > > >>> > > > > >>> > > > + sudo -E chroot --userspec=$( id -u ):$( id -g ) > > >>> > > > ${rootfs} \ > > >>> > > > + sh -c ' mkdir -p "/deb-src/${1}/${2}" && cd > > >>> > > > "/deb-src/${1}/${2}" && apt-get -y --download-only > > >>> > > > --only-source source "$2"="$3" ' download-src > > >>> > > > "${rootfs_distro}" "${src}" "${version}" > > >>> > > > + done > > >>> > > > + sudo -E -s <<'EOSUDO' > > >>> > > > + mountpoint -q "${rootfs}/deb-src" && \ > > >>> > > > + umount -l "${rootfs}/deb-src" > > >>> > > > + rm -rf "${rootfs}/deb-src" > > >>> > > > +EOSUDO > > >>> > > > + ) 9>"${DEBSRCDIR}/${rootfs_distro}.lock" > > >>> > > > +} > > >>> > > > + > > >>> > > > deb_dl_dir_import() { > > >>> > > > export pc="${DEBDIR}/${2}" > > >>> > > > export rootfs="${1}" > > >>> > > > diff --git a/meta/classes/image.bbclass > > >>> > > > b/meta/classes/image.bbclass index 6b04c0a..fcaebd6 100644 > > >>> > > > --- a/meta/classes/image.bbclass > > >>> > > > +++ b/meta/classes/image.bbclass > > >>> > > > @@ -63,7 +63,7 @@ image_do_mounts() { > > >>> > > > } > > >>> > > > > > >>> > > > ROOTFSDIR = "${IMAGE_ROOTFS}" > > >>> > > > -ROOTFS_FEATURES += "clean-package-cache > > >>> > > > generate-manifest" +ROOTFS_FEATURES += > > >>> > > > "clean-package-cache generate-manifest cach-deb-src" > > >>> > > > ROOTFS_PACKAGES += "${IMAGE_PREINSTALL} ${IMAGE_INSTALL}" > > >>> > > > ROOTFS_MANIFEST_DEPLOY_DIR ?= "${DEPLOY_DIR_IMAGE}" > > >>> > > > diff --git a/meta/classes/rootfs.bbclass > > >>> > > > b/meta/classes/rootfs.bbclass index cee358c..ee57989 > > >>> > > > 100644 --- a/meta/classes/rootfs.bbclass > > >>> > > > +++ b/meta/classes/rootfs.bbclass > > >>> > > > @@ -185,6 +185,14 @@ python do_rootfs_install() { > > >>> > > > } > > >>> > > > addtask rootfs_install before do_rootfs_postprocess after > > >>> > > > do_unpack > > >>> > > > +ROOTFS_POSTPROCESS_COMMAND += > > >>> > > > "${@bb.utils.contains('ROOTFS_FEATURES', 'cache-deb-src', > > >>> > > > 'cache_deb_src', '', d)}" +cache_deb_src() { > > >>> > > > + rootfs_install_resolvconf > > >>> > > > + deb_dl_dir_import ${ROOTFSDIR} ${ROOTFS_DISTRO} > > >>> > > > + debsrc_download ${ROOTFSDIR} ${ROOTFS_DISTRO} > > >>> > > > + rootfs_install_clean_files > > >>> > > > +} > > >>> > > > + > > >>> > > > ROOTFS_POSTPROCESS_COMMAND += > > >>> > > > "${@bb.utils.contains('ROOTFS_FEATURES', > > >>> > > > 'clean-package-cache', > > >>> > > > 'rootfs_postprocess_clean_package_cache', '', d)}" > > >>> > > > rootfs_postprocess_clean_package_cache() { sudo -E chroot > > >>> > > > '${ROOTFSDIR}' \ > > >>> > > > > >>> > > > > >>> > > > >>> > > > -- > > > You received this message because you are subscribed to the > > > Google > > Groups "isar-users" group. > > > To unsubscribe from this group and stop receiving emails from it, > > > send > > an email to isar-users+...@googlegroups.com. > > > To view this discussion on the web visit > > https://groups.google.com/d/msgid/isar-users/f6faa39e-fc03-46e0-99e0-6b08d09a8d4b%40googlegroups.com > > . > > >