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. 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}' \ > > > > > > > > > >