[PATCH v5 00/13] Deb-src caching implementation

[PATCH v5 00/13] Deb-src caching implementation

[Vijai Kumar K]

Changes in v5:
 The major changes in this series are as below,
    - (P2) Handle cases where HOST_DISTRO!=DISTRO. This avoids mixing
of debs from different distro which inturn helps the src caching logic.
    - (P5) Avoid downloading package from other builds.
    - (P8) Add a reprepro based sanity test to check if the repo contains
the sources for all the debs.
    - (P9) Introduce a new variable BASE_REPO_FEATURES which provides means
to enable or disable various base-apt features. (cache-deb-src for now)
    - Some fixes in (P12 & P13)
Also, addressed some review comments from Henning.
Git Tree: https://github.com/vj-kumar/isar/tree/vijai/debsrc5

Changes in v4:
 - Use <source package>=<version> format instead of just using <packagename>
to download the right version of source package.

Changes in v3:
 - Take care of non-existent downloads/deb-src directory.

Changes in v2:
 - Introduced additional patch to cache deb src
 - Rebased on top of henning/staging4 tree


Vijai Kumar K (13):
  rootfs: Make rootfs finalize a separate task
  deb-dl-dir: Cache host distro debs separately
  meta: cache deb srcs as part of postprocessing
  deb-dl-dir: Make debsrc_download faster
  deb-dl-dir: Download files only belonging to the current image
  deb-dl-dir: Factor out the mounting part
  deb-dl-dir: Fix skipping of removed files
  repository: Add a sanity test to check missing sources
  base-apt: Introduce BASE_REPO_FEATURES
  repository: Fix failures due to missing section
  scripts/ci_build.sh: Enable deb-src caching
  rootfs: Fix possible overwrite of existing resolv.conf
  rootfs: Handle failures when postprocess is rerun

 meta-isar/conf/local.conf.sample              |  4 ++
 meta/classes/deb-dl-dir.bbclass               | 72 ++++++++++++++++++-
 meta/classes/dpkg-gbp.bbclass                 |  8 ++-
 meta/classes/dpkg.bbclass                     |  8 ++-
 meta/classes/image-tools-extension.bbclass    |  4 +-
 meta/classes/image.bbclass                    | 43 ++++++++++-
 meta/classes/repository.bbclass               | 13 +++-
 meta/classes/rootfs.bbclass                   | 62 ++++++----------
 .../isar-bootstrap/isar-bootstrap.inc         | 12 +++-
 meta/recipes-devtools/base-apt/base-apt.bb    |  3 +
 scripts/ci_build.sh                           |  1 +
 11 files changed, 177 insertions(+), 53 deletions(-)

-- 
2.17.1

[PATCH v5 01/13] rootfs: Make rootfs finalize a separate task

[Vijai Kumar K]

With the current implementation it is difficult to append a
postprocess function which requires a chroot environment.
For example, to add a postprocess function which runs apt-get to
download all source of packages installed in the target.

rootfs_postprocess_finalize is not actually an optional feature
but instead a necessary cleanup function for image class.
So, move the implementation to image class and make it as a task.

Signed-off-by: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com>
---
 meta/classes/image.bbclass  | 41 ++++++++++++++++++++++++++++++++++++-
 meta/classes/rootfs.bbclass | 39 -----------------------------------
 2 files changed, 40 insertions(+), 40 deletions(-)

diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index 0150f27..6b04c0a 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 finalize-rootfs generate-manifest"
+ROOTFS_FEATURES += "clean-package-cache generate-manifest"
 ROOTFS_PACKAGES += "${IMAGE_PREINSTALL} ${IMAGE_INSTALL}"
 ROOTFS_MANIFEST_DEPLOY_DIR ?= "${DEPLOY_DIR_IMAGE}"
 
@@ -173,5 +173,44 @@ python do_deploy() {
 }
 addtask deploy before do_build after do_image
 
+do_rootfs_finalize() {
+    sudo -s <<'EOSUDO'
+        test -e "${ROOTFSDIR}/chroot-setup.sh" && \
+            "${ROOTFSDIR}/chroot-setup.sh" "cleanup" "${ROOTFSDIR}"
+        rm -f "${ROOTFSDIR}/chroot-setup.sh"
+
+        test ! -e "${ROOTFSDIR}/usr/share/doc/qemu-user-static" && \
+            find "${ROOTFSDIR}/usr/bin" \
+                -maxdepth 1 -name 'qemu-*-static' -type f -delete
+
+        mountpoint -q '${ROOTFSDIR}/isar-apt' && \
+            umount -l ${ROOTFSDIR}/isar-apt
+        rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/isar-apt
+
+        mountpoint -q '${ROOTFSDIR}/base-apt' && \
+            umount -l ${ROOTFSDIR}/base-apt
+        rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/base-apt
+
+        mountpoint -q '${ROOTFSDIR}/dev' && \
+            umount -l ${ROOTFSDIR}/dev
+        mountpoint -q '${ROOTFSDIR}/sys' && \
+            umount -l ${ROOTFSDIR}/proc
+        mountpoint -q '${ROOTFSDIR}/sys' && \
+            umount -l ${ROOTFSDIR}/sys
+
+        rm -f "${ROOTFSDIR}/etc/apt/apt.conf.d/55isar-fallback.conf"
+
+        rm -f "${ROOTFSDIR}/etc/apt/sources.list.d/isar-apt.list"
+        rm -f "${ROOTFSDIR}/etc/apt/preferences.d/isar-apt"
+        rm -f "${ROOTFSDIR}/etc/apt/sources.list.d/base-apt.list"
+
+        mv "${ROOTFSDIR}/etc/apt/sources-list" \
+            "${ROOTFSDIR}/etc/apt/sources.list.d/bootstrap.list"
+
+        rm -f "${ROOTFSDIR}/etc/apt/sources-list"
+EOSUDO
+}
+addtask rootfs_finalize before do_rootfs after do_rootfs_postprocess
+
 # Last so that the image type can overwrite tasks if needed
 inherit ${IMAGE_TYPE}
diff --git a/meta/classes/rootfs.bbclass b/meta/classes/rootfs.bbclass
index 806e824..8bb003d 100644
--- a/meta/classes/rootfs.bbclass
+++ b/meta/classes/rootfs.bbclass
@@ -201,45 +201,6 @@ rootfs_generate_manifest () {
         ${ROOTFS_MANIFEST_DEPLOY_DIR}/"${PF}".manifest
 }
 
-ROOTFS_POSTPROCESS_COMMAND += "${@bb.utils.contains('ROOTFS_FEATURES', 'finalize-rootfs', 'rootfs_postprocess_finalize', '', d)}"
-rootfs_postprocess_finalize() {
-    sudo -s <<'EOSUDO'
-        test -e "${ROOTFSDIR}/chroot-setup.sh" && \
-            "${ROOTFSDIR}/chroot-setup.sh" "cleanup" "${ROOTFSDIR}"
-        rm -f "${ROOTFSDIR}/chroot-setup.sh"
-
-        test ! -e "${ROOTFSDIR}/usr/share/doc/qemu-user-static" && \
-            find "${ROOTFSDIR}/usr/bin" \
-                -maxdepth 1 -name 'qemu-*-static' -type f -delete
-
-        mountpoint -q '${ROOTFSDIR}/isar-apt' && \
-            umount -l ${ROOTFSDIR}/isar-apt
-        rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/isar-apt
-
-        mountpoint -q '${ROOTFSDIR}/base-apt' && \
-            umount -l ${ROOTFSDIR}/base-apt
-        rmdir --ignore-fail-on-non-empty ${ROOTFSDIR}/base-apt
-
-        mountpoint -q '${ROOTFSDIR}/dev' && \
-            umount -l ${ROOTFSDIR}/dev
-        mountpoint -q '${ROOTFSDIR}/sys' && \
-            umount -l ${ROOTFSDIR}/proc
-        mountpoint -q '${ROOTFSDIR}/sys' && \
-            umount -l ${ROOTFSDIR}/sys
-
-        rm -f "${ROOTFSDIR}/etc/apt/apt.conf.d/55isar-fallback.conf"
-
-        rm -f "${ROOTFSDIR}/etc/apt/sources.list.d/isar-apt.list"
-        rm -f "${ROOTFSDIR}/etc/apt/preferences.d/isar-apt"
-        rm -f "${ROOTFSDIR}/etc/apt/sources.list.d/base-apt.list"
-
-        mv "${ROOTFSDIR}/etc/apt/sources-list" \
-            "${ROOTFSDIR}/etc/apt/sources.list.d/bootstrap.list"
-
-        rm -f "${ROOTFSDIR}/etc/apt/sources-list"
-EOSUDO
-}
-
 do_rootfs_postprocess[vardeps] = "${ROOTFS_POSTPROCESS_COMMAND}"
 python do_rootfs_postprocess() {
     # Take care that its correctly mounted:
-- 
2.17.1

[PATCH v5 02/13] deb-dl-dir: Cache host distro debs separately

[Vijai Kumar K]

In case of targets where HOST_DISTRO!=DISTRO, like rpi-stretch,
we were still caching the debs from both the distros into the same
DEBDIR/DISTRO directory. With this change, HOST_DISTRO would be
cached in the relevant subdirectory and avoids mixing of debs from
two separate distros.

Signed-off-by: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com>
---
 meta/classes/deb-dl-dir.bbclass                     |  4 ++--
 meta/classes/dpkg-gbp.bbclass                       |  8 ++++++--
 meta/classes/dpkg.bbclass                           |  8 ++++++--
 meta/classes/image-tools-extension.bbclass          |  4 ++--
 meta/classes/rootfs.bbclass                         |  4 ++--
 meta/recipes-core/isar-bootstrap/isar-bootstrap.inc | 12 ++++++++++--
 6 files changed, 28 insertions(+), 12 deletions(-)

diff --git a/meta/classes/deb-dl-dir.bbclass b/meta/classes/deb-dl-dir.bbclass
index e996703..29a3d67 100644
--- a/meta/classes/deb-dl-dir.bbclass
+++ b/meta/classes/deb-dl-dir.bbclass
@@ -6,7 +6,7 @@
 inherit repository
 
 deb_dl_dir_import() {
-    export pc="${DEBDIR}/${DISTRO}/"
+    export pc="${DEBDIR}/${2}"
     export rootfs="${1}"
     [ ! -d "${pc}" ] && return 0
     sudo mkdir -p "${rootfs}"/var/cache/apt/archives/
@@ -20,7 +20,7 @@ deb_dl_dir_import() {
 }
 
 deb_dl_dir_export() {
-    export pc="${DEBDIR}/${DISTRO}/"
+    export pc="${DEBDIR}/${2}"
     export rootfs="${1}"
     mkdir -p "${pc}"
     flock "${pc}".lock -c '
diff --git a/meta/classes/dpkg-gbp.bbclass b/meta/classes/dpkg-gbp.bbclass
index afa1e19..ba5c3eb 100644
--- a/meta/classes/dpkg-gbp.bbclass
+++ b/meta/classes/dpkg-gbp.bbclass
@@ -12,11 +12,15 @@ GBP_EXTRA_OPTIONS ?= "--git-pristine-tar"
 
 do_install_builddeps_append() {
     dpkg_do_mounts
-    deb_dl_dir_import "${BUILDCHROOT_DIR}"
+    distro="${DISTRO}"
+    if [ ${ISAR_CROSS_COMPILE} -eq 1 ]; then
+       distro="${HOST_DISTRO}"
+    fi
+    deb_dl_dir_import "${BUILDCHROOT_DIR}" "${distro}"
     sudo -E chroot ${BUILDCHROOT_DIR} \
         apt-get install -y -o Debug::pkgProblemResolver=yes \
                         --no-install-recommends --download-only ${GBP_DEPENDS}
-    deb_dl_dir_export "${BUILDCHROOT_DIR}"
+    deb_dl_dir_export "${BUILDCHROOT_DIR}" "${distro}"
     sudo -E chroot ${BUILDCHROOT_DIR} \
         apt-get install -y -o Debug::pkgProblemResolver=yes \
                         --no-install-recommends ${GBP_DEPENDS}
diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
index 585365e..53862b5 100644
--- a/meta/classes/dpkg.bbclass
+++ b/meta/classes/dpkg.bbclass
@@ -7,10 +7,14 @@ inherit dpkg-base
 do_install_builddeps() {
     dpkg_do_mounts
     E="${@ isar_export_proxies(d)}"
-    deb_dl_dir_import "${BUILDCHROOT_DIR}"
+    distro="${DISTRO}"
+    if [ ${ISAR_CROSS_COMPILE} -eq 1 ]; then
+       distro="${HOST_DISTRO}"
+    fi
+    deb_dl_dir_import "${BUILDCHROOT_DIR}" "${distro}"
     sudo -E chroot ${BUILDCHROOT_DIR} /isar/deps.sh \
         ${PP}/${PPS} ${DISTRO_ARCH} --download-only
-    deb_dl_dir_export "${BUILDCHROOT_DIR}"
+    deb_dl_dir_export "${BUILDCHROOT_DIR}" "${distro}"
     sudo -E chroot ${BUILDCHROOT_DIR} /isar/deps.sh \
         ${PP}/${PPS} ${DISTRO_ARCH}
     dpkg_undo_mounts
diff --git a/meta/classes/image-tools-extension.bbclass b/meta/classes/image-tools-extension.bbclass
index 6590ee7..0b067ff 100644
--- a/meta/classes/image-tools-extension.bbclass
+++ b/meta/classes/image-tools-extension.bbclass
@@ -25,7 +25,7 @@ do_install_imager_deps() {
     buildchroot_do_mounts
 
     E="${@ isar_export_proxies(d)}"
-    deb_dl_dir_import ${BUILDCHROOT_DIR}
+    deb_dl_dir_import ${BUILDCHROOT_DIR} ${DISTRO}
     sudo -E chroot ${BUILDCHROOT_DIR} sh -c ' \
         apt-get update \
             -o Dir::Etc::SourceList="sources.list.d/isar-apt.list" \
@@ -35,7 +35,7 @@ do_install_imager_deps() {
             --allow-unauthenticated --allow-downgrades --download-only install \
             ${IMAGER_INSTALL}'
 
-    deb_dl_dir_export ${BUILDCHROOT_DIR}
+    deb_dl_dir_export ${BUILDCHROOT_DIR} ${DISTRO}
     sudo -E chroot ${BUILDCHROOT_DIR} sh -c ' \
         apt-get -o Debug::pkgProblemResolver=yes --no-install-recommends -y \
             --allow-unauthenticated --allow-downgrades install \
diff --git a/meta/classes/rootfs.bbclass b/meta/classes/rootfs.bbclass
index 8bb003d..cee358c 100644
--- a/meta/classes/rootfs.bbclass
+++ b/meta/classes/rootfs.bbclass
@@ -115,7 +115,7 @@ rootfs_install_resolvconf() {
 ROOTFS_INSTALL_COMMAND += "rootfs_import_package_cache"
 rootfs_import_package_cache[weight] = "5"
 rootfs_import_package_cache() {
-    deb_dl_dir_import ${ROOTFSDIR}
+    deb_dl_dir_import ${ROOTFSDIR} ${ROOTFS_DISTRO}
 }
 
 ROOTFS_INSTALL_COMMAND += "rootfs_install_pkgs_download"
@@ -132,7 +132,7 @@ ROOTFS_INSTALL_COMMAND += "${ROOTFS_INSTALL_COMMAND_BEFORE_EXPORT}"
 ROOTFS_INSTALL_COMMAND += "rootfs_export_package_cache"
 rootfs_export_package_cache[weight] = "5"
 rootfs_export_package_cache() {
-    deb_dl_dir_export ${ROOTFSDIR}
+    deb_dl_dir_export ${ROOTFSDIR} ${ROOTFS_DISTRO}
 }
 
 ROOTFS_INSTALL_COMMAND += "${@ 'rootfs_install_clean_files' if (d.getVar('ROOTFS_CLEAN_FILES') or '').strip() else ''}"
diff --git a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
index da0d436..4061c86 100644
--- a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
+++ b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
@@ -247,7 +247,11 @@ isar_bootstrap() {
     export IS_HOST debootstrap_args E
     if [ ! -e "${DEPLOY_ISAR_BOOTSTRAP}" ]; then
         sudo rm -rf --one-file-system "${ROOTFSDIR}"
-        deb_dl_dir_import "${ROOTFSDIR}"
+        if [ "${IS_HOST}" ];then
+            deb_dl_dir_import "${ROOTFSDIR}" "${HOST_DISTRO}"
+        else
+            deb_dl_dir_import "${ROOTFSDIR}" "${DISTRO}"
+        fi
 
         sudo -E -s <<'EOSUDO'
             set -e
@@ -344,7 +348,11 @@ isar_bootstrap() {
             ln -Tfsr "${ROOTFSDIR}" "${DEPLOY_ISAR_BOOTSTRAP}"
 EOSUDO
     fi
-    deb_dl_dir_export "${ROOTFSDIR}"
+    if [ "${IS_HOST}" ];then
+        deb_dl_dir_export "${ROOTFSDIR}" "${HOST_DISTRO}"
+    else
+        deb_dl_dir_export "${ROOTFSDIR}" "${DISTRO}"
+    fi
 }
 
 CLEANFUNCS = "clean_deploy"
-- 
2.17.1

Re: [PATCH v5 02/13] deb-dl-dir: Cache host distro debs separately

[Henning Schild]

Hi Vijai,

you found one of the "known" issues of that implementation

https://groups.google.com/d/msg/isar-users/8djs0Thl0wI/n5Bo2hPeCgAJ

There i talk about "mixing" packages from many sources into one
base-apt under one "suite" like "buster".

In fact such a mix in the download cache is what you found here. But
maybe for the cache we can store everything plain in one folder. Given
that an "apt-get source" without a version would "unpack" only what it
found in its Sources.gz(s) ... which i would assume.

But i think what you suggest here should work as well. And there is
probably not much overlap ... packages with exactly the same versions
in multiple suites.

Henning

On Fri, 17 Apr 2020 15:00:29 +0530
Vijai Kumar K <vijaikumar.kanagarajan@gmail.com> wrote:

> In case of targets where HOST_DISTRO!=DISTRO, like rpi-stretch,
> we were still caching the debs from both the distros into the same
> DEBDIR/DISTRO directory. With this change, HOST_DISTRO would be
> cached in the relevant subdirectory and avoids mixing of debs from
> two separate distros.
> 
> Signed-off-by: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com>
> ---
>  meta/classes/deb-dl-dir.bbclass                     |  4 ++--
>  meta/classes/dpkg-gbp.bbclass                       |  8 ++++++--
>  meta/classes/dpkg.bbclass                           |  8 ++++++--
>  meta/classes/image-tools-extension.bbclass          |  4 ++--
>  meta/classes/rootfs.bbclass                         |  4 ++--
>  meta/recipes-core/isar-bootstrap/isar-bootstrap.inc | 12 ++++++++++--
>  6 files changed, 28 insertions(+), 12 deletions(-)
> 
> diff --git a/meta/classes/deb-dl-dir.bbclass
> b/meta/classes/deb-dl-dir.bbclass index e996703..29a3d67 100644
> --- a/meta/classes/deb-dl-dir.bbclass
> +++ b/meta/classes/deb-dl-dir.bbclass
> @@ -6,7 +6,7 @@
>  inherit repository
>  
>  deb_dl_dir_import() {
> -    export pc="${DEBDIR}/${DISTRO}/"
> +    export pc="${DEBDIR}/${2}"
>      export rootfs="${1}"
>      [ ! -d "${pc}" ] && return 0
>      sudo mkdir -p "${rootfs}"/var/cache/apt/archives/
> @@ -20,7 +20,7 @@ deb_dl_dir_import() {
>  }
>  
>  deb_dl_dir_export() {
> -    export pc="${DEBDIR}/${DISTRO}/"
> +    export pc="${DEBDIR}/${2}"
>      export rootfs="${1}"
>      mkdir -p "${pc}"
>      flock "${pc}".lock -c '
> diff --git a/meta/classes/dpkg-gbp.bbclass
> b/meta/classes/dpkg-gbp.bbclass index afa1e19..ba5c3eb 100644
> --- a/meta/classes/dpkg-gbp.bbclass
> +++ b/meta/classes/dpkg-gbp.bbclass
> @@ -12,11 +12,15 @@ GBP_EXTRA_OPTIONS ?= "--git-pristine-tar"
>  
>  do_install_builddeps_append() {
>      dpkg_do_mounts
> -    deb_dl_dir_import "${BUILDCHROOT_DIR}"
> +    distro="${DISTRO}"
> +    if [ ${ISAR_CROSS_COMPILE} -eq 1 ]; then
> +       distro="${HOST_DISTRO}"
> +    fi
> +    deb_dl_dir_import "${BUILDCHROOT_DIR}" "${distro}"
>      sudo -E chroot ${BUILDCHROOT_DIR} \
>          apt-get install -y -o Debug::pkgProblemResolver=yes \
>                          --no-install-recommends --download-only
> ${GBP_DEPENDS}
> -    deb_dl_dir_export "${BUILDCHROOT_DIR}"
> +    deb_dl_dir_export "${BUILDCHROOT_DIR}" "${distro}"
>      sudo -E chroot ${BUILDCHROOT_DIR} \
>          apt-get install -y -o Debug::pkgProblemResolver=yes \
>                          --no-install-recommends ${GBP_DEPENDS}
> diff --git a/meta/classes/dpkg.bbclass b/meta/classes/dpkg.bbclass
> index 585365e..53862b5 100644
> --- a/meta/classes/dpkg.bbclass
> +++ b/meta/classes/dpkg.bbclass
> @@ -7,10 +7,14 @@ inherit dpkg-base
>  do_install_builddeps() {
>      dpkg_do_mounts
>      E="${@ isar_export_proxies(d)}"
> -    deb_dl_dir_import "${BUILDCHROOT_DIR}"
> +    distro="${DISTRO}"
> +    if [ ${ISAR_CROSS_COMPILE} -eq 1 ]; then
> +       distro="${HOST_DISTRO}"
> +    fi
> +    deb_dl_dir_import "${BUILDCHROOT_DIR}" "${distro}"
>      sudo -E chroot ${BUILDCHROOT_DIR} /isar/deps.sh \
>          ${PP}/${PPS} ${DISTRO_ARCH} --download-only
> -    deb_dl_dir_export "${BUILDCHROOT_DIR}"
> +    deb_dl_dir_export "${BUILDCHROOT_DIR}" "${distro}"
>      sudo -E chroot ${BUILDCHROOT_DIR} /isar/deps.sh \
>          ${PP}/${PPS} ${DISTRO_ARCH}
>      dpkg_undo_mounts
> diff --git a/meta/classes/image-tools-extension.bbclass
> b/meta/classes/image-tools-extension.bbclass index 6590ee7..0b067ff
> 100644 --- a/meta/classes/image-tools-extension.bbclass
> +++ b/meta/classes/image-tools-extension.bbclass
> @@ -25,7 +25,7 @@ do_install_imager_deps() {
>      buildchroot_do_mounts
>  
>      E="${@ isar_export_proxies(d)}"
> -    deb_dl_dir_import ${BUILDCHROOT_DIR}
> +    deb_dl_dir_import ${BUILDCHROOT_DIR} ${DISTRO}
>      sudo -E chroot ${BUILDCHROOT_DIR} sh -c ' \
>          apt-get update \
>              -o Dir::Etc::SourceList="sources.list.d/isar-apt.list" \
> @@ -35,7 +35,7 @@ do_install_imager_deps() {
>              --allow-unauthenticated --allow-downgrades
> --download-only install \ ${IMAGER_INSTALL}'
>  
> -    deb_dl_dir_export ${BUILDCHROOT_DIR}
> +    deb_dl_dir_export ${BUILDCHROOT_DIR} ${DISTRO}
>      sudo -E chroot ${BUILDCHROOT_DIR} sh -c ' \
>          apt-get -o Debug::pkgProblemResolver=yes
> --no-install-recommends -y \ --allow-unauthenticated
> --allow-downgrades install \ diff --git a/meta/classes/rootfs.bbclass
> b/meta/classes/rootfs.bbclass index 8bb003d..cee358c 100644
> --- a/meta/classes/rootfs.bbclass
> +++ b/meta/classes/rootfs.bbclass
> @@ -115,7 +115,7 @@ rootfs_install_resolvconf() {
>  ROOTFS_INSTALL_COMMAND += "rootfs_import_package_cache"
>  rootfs_import_package_cache[weight] = "5"
>  rootfs_import_package_cache() {
> -    deb_dl_dir_import ${ROOTFSDIR}
> +    deb_dl_dir_import ${ROOTFSDIR} ${ROOTFS_DISTRO}
>  }
>  
>  ROOTFS_INSTALL_COMMAND += "rootfs_install_pkgs_download"
> @@ -132,7 +132,7 @@ ROOTFS_INSTALL_COMMAND +=
> "${ROOTFS_INSTALL_COMMAND_BEFORE_EXPORT}" ROOTFS_INSTALL_COMMAND +=
> "rootfs_export_package_cache" rootfs_export_package_cache[weight] =
> "5" rootfs_export_package_cache() {
> -    deb_dl_dir_export ${ROOTFSDIR}
> +    deb_dl_dir_export ${ROOTFSDIR} ${ROOTFS_DISTRO}
>  }
>  
>  ROOTFS_INSTALL_COMMAND += "${@ 'rootfs_install_clean_files' if
> (d.getVar('ROOTFS_CLEAN_FILES') or '').strip() else ''}" diff --git
> a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc
> b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc index
> da0d436..4061c86 100644 ---
> a/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc +++
> b/meta/recipes-core/isar-bootstrap/isar-bootstrap.inc @@ -247,7
> +247,11 @@ isar_bootstrap() { export IS_HOST debootstrap_args E if [
> ! -e "${DEPLOY_ISAR_BOOTSTRAP}" ]; then sudo rm -rf --one-file-system
> "${ROOTFSDIR}"
> -        deb_dl_dir_import "${ROOTFSDIR}"
> +        if [ "${IS_HOST}" ];then
> +            deb_dl_dir_import "${ROOTFSDIR}" "${HOST_DISTRO}"
> +        else
> +            deb_dl_dir_import "${ROOTFSDIR}" "${DISTRO}"
> +        fi
>  
>          sudo -E -s <<'EOSUDO'
>              set -e
> @@ -344,7 +348,11 @@ isar_bootstrap() {
>              ln -Tfsr "${ROOTFSDIR}" "${DEPLOY_ISAR_BOOTSTRAP}"
>  EOSUDO
>      fi
> -    deb_dl_dir_export "${ROOTFSDIR}"
> +    if [ "${IS_HOST}" ];then
> +        deb_dl_dir_export "${ROOTFSDIR}" "${HOST_DISTRO}"
> +    else
> +        deb_dl_dir_export "${ROOTFSDIR}" "${DISTRO}"
> +    fi
>  }
>  
>  CLEANFUNCS = "clean_deploy"

[PATCH v5 03/13] meta: cache deb srcs as part of postprocessing

[Vijai Kumar K]

Collect the deb sources of the corresponding deb binaries cached
in DEBDIR as part of image postprocess.

Signed-off-by: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com>
---
 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
+    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.
+        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
+
+        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}' \
-- 
2.17.1

Re: [PATCH v5 03/13] meta: cache deb srcs as part of postprocessing

[Henning Schild]

On Fri, 17 Apr 2020 15:00:30 +0530
Vijai Kumar K <vijaikumar.kanagarajan@gmail.com> wrote:

> Collect the deb sources of the corresponding deb binaries cached
> in DEBDIR as part of image postprocess.
> 
> Signed-off-by: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com>
> ---
>  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.

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.

> +    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.

> +        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.

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

Re: [PATCH v5 03/13] meta: cache deb srcs as part of postprocessing

[vijai kumar]

[-- Attachment #1.1: Type: text/plain, Size: 5449 bytes --]



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 <vijaikumar...@gmail.com <javascript:>> wrote: 
>
> > Collect the deb sources of the corresponding deb binaries cached 
> > in DEBDIR as part of image postprocess. 
> > 
> > Signed-off-by: Vijai Kumar K <Vijaikumar_...@mentor.com <javascript:>> 
> > --- 
> >  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. 
 

> 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.

> +    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
 

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

[-- Attachment #1.2: Type: text/html, Size: 8223 bytes --]

Re: [PATCH v5 03/13] meta: cache deb srcs as part of postprocessing

[Henning Schild]

Am Wed, 22 Apr 2020 02:57:13 -0700 (PDT)
schrieb vijai kumar <vijaikumar.kanagarajan@gmail.com>:

> 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 <vijaikumar...@gmail.com <javascript:>> wrote: 
> >  
> > > Collect the deb sources of the corresponding deb binaries cached 
> > > in DEBDIR as part of image postprocess. 
> > > 
> > > Signed-off-by: Vijai Kumar K <Vijaikumar_...@mentor.com
> > > <javascript:>> --- 
> > >  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?
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.

> > 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?
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.

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

Re: [PATCH v5 03/13] meta: cache deb srcs as part of postprocessing

[vijai kumar]

[-- Attachment #1.1: Type: text/plain, Size: 8500 bytes --]



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 <vijaikumar...@gmail.com <javascript:>>: 
>
> > 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 <vijaikumar...@gmail.com <javascript:>> wrote: 
> > >   
> > > > Collect the deb sources of the corresponding deb binaries cached 
> > > > in DEBDIR as part of image postprocess. 
> > > > 
> > > > Signed-off-by: Vijai Kumar K <Vijaikumar_...@mentor.com 
> > > > <javascript:>> --- 
> > > >  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}' \   
> > > 
> > >   
> > 
>
>

[-- Attachment #1.2: Type: text/html, Size: 13018 bytes --]

Re: [PATCH v5 03/13] meta: cache deb srcs as part of postprocessing

[vijai kumar]

[-- Attachment #1.1: Type: text/plain, Size: 8959 bytes --]



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 <vijaikumar...@gmail.com>: 
>>
>> > 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 <vijaikumar...@gmail.com <javascript:>> wrote: 
>> > >   
>> > > > Collect the deb sources of the corresponding deb binaries cached 
>> > > > in DEBDIR as part of image postprocess. 
>> > > > 
>> > > > Signed-off-by: Vijai Kumar K <Vijaikumar_...@mentor.com 
>> > > > <javascript:>> --- 
>> > > >  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}' \   
>> > > 
>> > >   
>> > 
>>
>>

[-- Attachment #1.2: Type: text/html, Size: 13506 bytes --]

Re: [PATCH v5 03/13] meta: cache deb srcs as part of postprocessing

[vijai kumar]

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
<vijaikumar.kanagarajan@gmail.com> 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 <vijaikumar...@gmail.com>:
>>>
>>> > 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 <vijaikumar...@gmail.com <javascript:>> wrote:
>>> > >
>>> > > > Collect the deb sources of the corresponding deb binaries cached
>>> > > > in DEBDIR as part of image postprocess.
>>> > > >
>>> > > > Signed-off-by: Vijai Kumar K <Vijaikumar_...@mentor.com
>>> > > > <javascript:>> ---
>>> > > >  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+unsubscribe@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.

Re: [PATCH v5 03/13] meta: cache deb srcs as part of postprocessing

[vijaikumar....@gmail.com]

[-- Attachment #1.1: Type: text/plain, Size: 9937 bytes --]

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
> <vijaikumar....@gmail.com> 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 <vijaikumar...@gmail.com>:
> >>>
> >>> > 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 <vijaikumar...@gmail.com <javascript:>> wrote:
> >>> > >
> >>> > > > Collect the deb sources of the corresponding deb binaries cached
> >>> > > > in DEBDIR as part of image postprocess.
> >>> > > >
> >>> > > > Signed-off-by: Vijai Kumar K <Vijaikumar_...@mentor.com
> >>> > > > <javascript:>> ---
> >>> > > > 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
> .
>

[-- Attachment #1.2: Type: text/html, Size: 15407 bytes --]

Re: [PATCH v5 03/13] meta: cache deb srcs as part of postprocessing

[Henning Schild]

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" <vijaikumar.kanagarajan@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
> > <vijaikumar....@gmail.com> 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 <vijaikumar...@gmail.com>:
> > >>>  
> > >>> > 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 <vijaikumar...@gmail.com <javascript:>> wrote:
> > >>> > >  
> > >>> > > > Collect the deb sources of the corresponding deb binaries
> > >>> > > > cached in DEBDIR as part of image postprocess.
> > >>> > > >
> > >>> > > > Signed-off-by: Vijai Kumar K <Vijaikumar_...@mentor.com
> > >>> > > > <javascript:>> ---
> > >>> > > > 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
> > .
> >  
> 

Re: [PATCH v5 03/13] meta: cache deb srcs as part of postprocessing

[vijaikumar....@gmail.com]

[-- Attachment #1.1: Type: text/plain, Size: 12152 bytes --]



On Friday, September 4, 2020 at 12:28:24 AM UTC+5:30 Henning Schild wrote:

> 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. 
>
>
No problem Hennnig. I will implement your initial review comments and send
out a new series. Hoping that other reviewers would be able to have a look 
at that.
Please chime in when you find time.

Thanks,
Vijai Kumar K

Henning 
>
> On Thu, 3 Sep 2020 04:43:13 -0700 (PDT) 
> "vijaikumar....@gmail.com" <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 
> > > <vijaikumar....@gmail.com> 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 <vijaikumar...@gmail.com>: 
> > > >>> 
> > > >>> > 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 <vijaikumar...@gmail.com <javascript:>> wrote: 
> > > >>> > > 
> > > >>> > > > Collect the deb sources of the corresponding deb binaries 
> > > >>> > > > cached in DEBDIR as part of image postprocess. 
> > > >>> > > > 
> > > >>> > > > Signed-off-by: Vijai Kumar K <Vijaikumar_...@mentor.com 
> > > >>> > > > <javascript:>> --- 
> > > >>> > > > 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 
> > > . 
> > > 
> > 
>
>

[-- Attachment #1.2: Type: text/html, Size: 19330 bytes --]

Re: [PATCH v5 03/13] meta: cache deb srcs as part of postprocessing

[Henning Schild]

On Fri, 17 Apr 2020 15:00:30 +0530
Vijai Kumar K <vijaikumar.kanagarajan@gmail.com> wrote:

> Collect the deb sources of the corresponding deb binaries cached
> in DEBDIR as part of image postprocess.
> 
> Signed-off-by: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com>
> ---
>  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
> +    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

Using that cache as input is not a good idea. It does not tell you what
you have installed. It might be overfull because of the download cache,
and it might be too empty because of cache cleanups.
You probably want to iterate of the list of installed packages, or
better that manifest we have.

Henning

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

Re: [PATCH v5 03/13] meta: cache deb srcs as part of postprocessing

[vijai kumar]

[-- Attachment #1.1: Type: text/plain, Size: 4962 bytes --]



On Wednesday, April 22, 2020 at 12:58:22 PM UTC+5:30, Henning Schild wrote:
>
> On Fri, 17 Apr 2020 15:00:30 +0530 
> Vijai Kumar K <vijaikumar...@gmail.com <javascript:>> wrote: 
>
> > Collect the deb sources of the corresponding deb binaries cached 
> > in DEBDIR as part of image postprocess. 
> > 
> > Signed-off-by: Vijai Kumar K <Vijaikumar_...@mentor.com <javascript:>> 
> > --- 
> >  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 
> > +    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 
>
> Using that cache as input is not a good idea. It does not tell you what 
> you have installed. It might be overfull because of the download cache, 
> and it might be too empty because of cache cleanups. 
> You probably want to iterate of the list of installed packages, or 
> better that manifest we have. 
>


At mid implementation I too had that thought. Please see my comment
in [1]

[1] https://groups.google.com/d/msg/isar-users/quJz5ikjQPs/qLw2-FoEAwAJ



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

[-- Attachment #1.2: Type: text/html, Size: 7137 bytes --]

[PATCH v5 04/13] deb-dl-dir: Make debsrc_download faster

[Vijai Kumar K]

Eventhough apt-get source skips redownloading of files, it is still
slow and takes a lot of time. Instead, lookup if the dsc file is already
present in the cache and skip based on it.

Signed-off-by: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com>
---
 meta/classes/deb-dl-dir.bbclass | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/meta/classes/deb-dl-dir.bbclass b/meta/classes/deb-dl-dir.bbclass
index 472b9fe..9399741 100644
--- a/meta/classes/deb-dl-dir.bbclass
+++ b/meta/classes/deb-dl-dir.bbclass
@@ -32,6 +32,9 @@ EOSUDO
         if [ -z "${src}" ];then
             src="$( dpkg-deb --show --showformat '${Package}' "${package}" )"
         fi
+        # Strip epoch, if any, from version.
+        local dscfile=$(find "${DEBSRCDIR}"/"${rootfs_distro}" -name "${src}_${version#*:}.dsc")
+        [ -z "$dscfile" ] || continue
 
         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}"
-- 
2.17.1

Re: [PATCH v5 04/13] deb-dl-dir: Make debsrc_download faster

[Henning Schild]

On Fri, 17 Apr 2020 15:00:31 +0530
Vijai Kumar K <vijaikumar.kanagarajan@gmail.com> wrote:

> Eventhough apt-get source skips redownloading of files, it is still
> slow and takes a lot of time. Instead, lookup if the dsc file is
> already present in the cache and skip based on it.
> 
> Signed-off-by: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com>
> ---
>  meta/classes/deb-dl-dir.bbclass | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/meta/classes/deb-dl-dir.bbclass
> b/meta/classes/deb-dl-dir.bbclass index 472b9fe..9399741 100644
> --- a/meta/classes/deb-dl-dir.bbclass
> +++ b/meta/classes/deb-dl-dir.bbclass
> @@ -32,6 +32,9 @@ EOSUDO
>          if [ -z "${src}" ];then
>              src="$( dpkg-deb --show --showformat '${Package}'
> "${package}" )" fi
> +        # Strip epoch, if any, from version.
> +        local dscfile=$(find "${DEBSRCDIR}"/"${rootfs_distro}" -name
> "${src}_${version#*:}.dsc")

Is that reliable, or might you end up exiting too early. You process
that "version" quite a bit, it needs to remain a unique identifier. So
you do not skip "foo-1.2.3_r1@deb~42" only because "foo-1.2.3" was
there. I made that up but such a version is allowed to have all sorts
of funny chars.

> +        [ -z "$dscfile" ] || continue

I personally would say "-n" && continue, might be a matter of taste.

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}"

Re: [PATCH v5 04/13] deb-dl-dir: Make debsrc_download faster

[vijai kumar]

[-- Attachment #1.1: Type: text/plain, Size: 2524 bytes --]



On Wednesday, April 22, 2020 at 12:45:42 PM UTC+5:30, Henning Schild wrote:
>
> On Fri, 17 Apr 2020 15:00:31 +0530 
> Vijai Kumar K <vijaikumar...@gmail.com <javascript:>> wrote: 
>
> > Eventhough apt-get source skips redownloading of files, it is still 
> > slow and takes a lot of time. Instead, lookup if the dsc file is 
> > already present in the cache and skip based on it. 
> > 
> > Signed-off-by: Vijai Kumar K <Vijaikumar_...@mentor.com <javascript:>> 
> > --- 
> >  meta/classes/deb-dl-dir.bbclass | 3 +++ 
> >  1 file changed, 3 insertions(+) 
> > 
> > diff --git a/meta/classes/deb-dl-dir.bbclass 
> > b/meta/classes/deb-dl-dir.bbclass index 472b9fe..9399741 100644 
> > --- a/meta/classes/deb-dl-dir.bbclass 
> > +++ b/meta/classes/deb-dl-dir.bbclass 
> > @@ -32,6 +32,9 @@ EOSUDO 
> >          if [ -z "${src}" ];then 
> >              src="$( dpkg-deb --show --showformat '${Package}' 
> > "${package}" )" fi 
> > +        # Strip epoch, if any, from version. 
> > +        local dscfile=$(find "${DEBSRCDIR}"/"${rootfs_distro}" -name 
> > "${src}_${version#*:}.dsc") 
>
> Is that reliable, or might you end up exiting too early. You process 
> that "version" quite a bit, it needs to remain a unique identifier. So 
> you do not skip "foo-1.2.3_r1@deb~42" only because "foo-1.2.3" was 
> there. I made that up but such a version is allowed to have all sorts 
> of funny chars. 
>

As per the Debian policy manual,  version format is 
[epoch:]upstream_version[-debian_revision].
While downloading using apt-get source <source pkg>=<version>, I observed 
that epoch is needed
for downloading the source correctly. However the downloaded dsc file does 
not contain epoch. Hence
this stripping while checking for the presence of the corresponding dsc 
file.

Also, Debian policy manual[1] states that . + - ~ (full stop, plus, hyphen, 
tilde) are the valid values
for use in upstream-version part and + . ~ (plus, full stop, tilde) for 
debian-revision part.

So, this epoch stripping should be fine.

[1] 
https://www.debian.org/doc/debian-policy/ch-controlfields.html#s-f-version


> > +        [ -z "$dscfile" ] || continue 
>
> I personally would say "-n" && continue, might be a matter of taste. 

 
>
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}" 
>
>

[-- Attachment #1.2: Type: text/html, Size: 6772 bytes --]

Re: [PATCH v5 04/13] deb-dl-dir: Make debsrc_download faster

[Henning Schild]

Am Wed, 22 Apr 2020 03:11:11 -0700 (PDT)
schrieb vijai kumar <vijaikumar.kanagarajan@gmail.com>:

> On Wednesday, April 22, 2020 at 12:45:42 PM UTC+5:30, Henning Schild
> wrote:
> >
> > On Fri, 17 Apr 2020 15:00:31 +0530 
> > Vijai Kumar K <vijaikumar...@gmail.com <javascript:>> wrote: 
> >  
> > > Eventhough apt-get source skips redownloading of files, it is
> > > still slow and takes a lot of time. Instead, lookup if the dsc
> > > file is already present in the cache and skip based on it. 
> > > 
> > > Signed-off-by: Vijai Kumar K <Vijaikumar_...@mentor.com
> > > <javascript:>> --- 
> > >  meta/classes/deb-dl-dir.bbclass | 3 +++ 
> > >  1 file changed, 3 insertions(+) 
> > > 
> > > diff --git a/meta/classes/deb-dl-dir.bbclass 
> > > b/meta/classes/deb-dl-dir.bbclass index 472b9fe..9399741 100644 
> > > --- a/meta/classes/deb-dl-dir.bbclass 
> > > +++ b/meta/classes/deb-dl-dir.bbclass 
> > > @@ -32,6 +32,9 @@ EOSUDO 
> > >          if [ -z "${src}" ];then 
> > >              src="$( dpkg-deb --show --showformat '${Package}' 
> > > "${package}" )" fi 
> > > +        # Strip epoch, if any, from version. 
> > > +        local dscfile=$(find "${DEBSRCDIR}"/"${rootfs_distro}"
> > > -name "${src}_${version#*:}.dsc")   
> >
> > Is that reliable, or might you end up exiting too early. You
> > process that "version" quite a bit, it needs to remain a unique
> > identifier. So you do not skip "foo-1.2.3_r1@deb~42" only because
> > "foo-1.2.3" was there. I made that up but such a version is allowed
> > to have all sorts of funny chars. 
> >  
> 
> As per the Debian policy manual,  version format is 
> [epoch:]upstream_version[-debian_revision].
> While downloading using apt-get source <source pkg>=<version>, I
> observed that epoch is needed
> for downloading the source correctly. However the downloaded dsc file
> does not contain epoch. Hence
> this stripping while checking for the presence of the corresponding
> dsc file.
> 
> Also, Debian policy manual[1] states that . + - ~ (full stop, plus,
> hyphen, tilde) are the valid values
> for use in upstream-version part and + . ~ (plus, full stop, tilde)
> for debian-revision part.
> 
> So, this epoch stripping should be fine.

Thanks for looking into that. I remember running into a very nasty
cornercase once. 

debian-revision
... It is optional; if it isn’t present then the upstream_version may
not contain a hyphen ...

But if you are not cutting the string at hyphens you might be fine.

Henning


> [1] 
> https://www.debian.org/doc/debian-policy/ch-controlfields.html#s-f-version
> 
> 
> > > +        [ -z "$dscfile" ] || continue   
> >
> > I personally would say "-n" && continue, might be a matter of
> > taste.   
> 
>  
> >  
> 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}"   
> >
> >  
> 

[PATCH v5 05/13] deb-dl-dir: Download files only belonging to the current image

[Vijai Kumar K]

Avoid downloading deb-srcs for debs cached from other image builds.
One way to ensure that is to see if the package is present in the dpkg
status file.

Signed-off-by: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com>
---
 meta/classes/deb-dl-dir.bbclass | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/meta/classes/deb-dl-dir.bbclass b/meta/classes/deb-dl-dir.bbclass
index 9399741..b3f4842 100644
--- a/meta/classes/deb-dl-dir.bbclass
+++ b/meta/classes/deb-dl-dir.bbclass
@@ -5,6 +5,15 @@
 
 inherit repository
 
+check_in_rootfs() {
+    local package="$( dpkg-deb --show --showformat '${Package}' "${1}" )"
+    local output="$( grep -hs "^Package: ${package}" \
+            "${IMAGE_ROOTFS}"/var/lib/dpkg/status \
+            "${BUILDCHROOT_HOST_DIR}"/var/lib/dpkg/status \
+            "${BUILDCHROOT_TARGET_DIR}"/var/lib/dpkg/status )"
+    [ -z "${output}" ] && return 1 || return 0
+}
+
 debsrc_download() {
     export rootfs="$1"
     export rootfs_distro="$2"
@@ -18,6 +27,7 @@ debsrc_download() {
     mount --bind "${DEBSRCDIR}" "${rootfs}/deb-src"
 EOSUDO
     find "${rootfs}/var/cache/apt/archives/" -maxdepth 1 -type f -iname '*\.deb' | while read package; do
+        check_in_rootfs "${package}" || continue
         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.
-- 
2.17.1

Re: [PATCH v5 05/13] deb-dl-dir: Download files only belonging to the current image

[Henning Schild]

On Fri, 17 Apr 2020 15:00:32 +0530
Vijai Kumar K <vijaikumar.kanagarajan@gmail.com> wrote:

> Avoid downloading deb-srcs for debs cached from other image builds.
> One way to ensure that is to see if the package is present in the dpkg
> status file.

Not sure i understand what it is doing/avoiding. Maybe you can go into
more detail with an example.

> Signed-off-by: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com>
> ---
>  meta/classes/deb-dl-dir.bbclass | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/meta/classes/deb-dl-dir.bbclass
> b/meta/classes/deb-dl-dir.bbclass index 9399741..b3f4842 100644
> --- a/meta/classes/deb-dl-dir.bbclass
> +++ b/meta/classes/deb-dl-dir.bbclass
> @@ -5,6 +5,15 @@
>  
>  inherit repository
>
> +check_in_rootfs() {

I am confused by the logic again. And maybe the name. We already have
repo_contains_package which suggests a "boolean" answer, should this
one be "rootfs_contains_package"? Or maybe "has_package_installed"

> +    local package="$( dpkg-deb --show --showformat '${Package}'
> "${1}" )"
> +    local output="$( grep -hs "^Package: ${package}" \
> +            "${IMAGE_ROOTFS}"/var/lib/dpkg/status \
> +            "${BUILDCHROOT_HOST_DIR}"/var/lib/dpkg/status \
> +            "${BUILDCHROOT_TARGET_DIR}"/var/lib/dpkg/status )"

I think you should chroot in and ask "dpkg -i", doing this kind of
guessing on the outside is error-prone.

> +    [ -z "${output}" ] && return 1 || return 0

Again confused, in shell boolean "0" means true and anything else means
false.

return [ -n "${output}" ]

> +}
> +
>  debsrc_download() {
>      export rootfs="$1"
>      export rootfs_distro="$2"
> @@ -18,6 +27,7 @@ debsrc_download() {
>      mount --bind "${DEBSRCDIR}" "${rootfs}/deb-src"
>  EOSUDO
>      find "${rootfs}/var/cache/apt/archives/" -maxdepth 1 -type f
> -iname '*\.deb' | while read package; do
> +        check_in_rootfs "${package}" || continue

This reads like you skip the download when you found package to _not_
be "in_rootfs".

Henning

>          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.

Re: [PATCH v5 05/13] deb-dl-dir: Download files only belonging to the current image

[vijai kumar]

[-- Attachment #1.1: Type: text/plain, Size: 3398 bytes --]



On Wednesday, April 22, 2020 at 12:55:19 PM UTC+5:30, Henning Schild wrote:
>
> On Fri, 17 Apr 2020 15:00:32 +0530 
> Vijai Kumar K <vijaikumar...@gmail.com <javascript:>> wrote: 
>
> > Avoid downloading deb-srcs for debs cached from other image builds. 
> > One way to ensure that is to see if the package is present in the dpkg 
> > status file. 
>
> Not sure i understand what it is doing/avoiding. Maybe you can go into 
> more detail with an example. 
>

We are looping over the entries in DEBDIR/deb, It would contain packages 
from
all former builds. To make sure we take only those are used in the image, 
we check
the status file. While handling this I had a thought why not to use the 
status file as the
source directly, but P7 in this series changed that. Not all is available 
in status file, 
only those which are installed. There might be chances that something is 
removed later
on like the localepurge in meta/classes/image-locales-extension.bbclass. 
Those does not
get captured in the status file or the manifest and hence the dpkg log 
check in P7.


> > Signed-off-by: Vijai Kumar K <Vijaikumar_...@mentor.com <javascript:>> 
> > --- 
> >  meta/classes/deb-dl-dir.bbclass | 10 ++++++++++ 
> >  1 file changed, 10 insertions(+) 
> > 
> > diff --git a/meta/classes/deb-dl-dir.bbclass 
> > b/meta/classes/deb-dl-dir.bbclass index 9399741..b3f4842 100644 
> > --- a/meta/classes/deb-dl-dir.bbclass 
> > +++ b/meta/classes/deb-dl-dir.bbclass 
> > @@ -5,6 +5,15 @@ 
> >   
> >  inherit repository 
> > 
> > +check_in_rootfs() { 
>
> I am confused by the logic again. And maybe the name. We already have 
> repo_contains_package which suggests a "boolean" answer, should this 
> one be "rootfs_contains_package"? Or maybe "has_package_installed" 
>

I have to admit, on seeing this back, it is a bit confusing, I will fix 
these naming.
 

>
> > +    local package="$( dpkg-deb --show --showformat '${Package}' 
> > "${1}" )" 
> > +    local output="$( grep -hs "^Package: ${package}" \ 
> > +            "${IMAGE_ROOTFS}"/var/lib/dpkg/status \ 
> > +            "${BUILDCHROOT_HOST_DIR}"/var/lib/dpkg/status \ 
> > +            "${BUILDCHROOT_TARGET_DIR}"/var/lib/dpkg/status )" 
>
> I think you should chroot in and ask "dpkg -i", doing this kind of 
> guessing on the outside is error-prone. 
>
> > +    [ -z "${output}" ] && return 1 || return 0 
>
> Again confused, in shell boolean "0" means true and anything else means 
> false. 
>

Sorry. Confusing indeed. I will use the below. Thanks.
 

>
> return [ -n "${output}" ] 
>
> > +} 
> > + 
> >  debsrc_download() { 
> >      export rootfs="$1" 
> >      export rootfs_distro="$2" 
> > @@ -18,6 +27,7 @@ debsrc_download() { 
> >      mount --bind "${DEBSRCDIR}" "${rootfs}/deb-src" 
> >  EOSUDO 
> >      find "${rootfs}/var/cache/apt/archives/" -maxdepth 1 -type f 
> > -iname '*\.deb' | while read package; do 
> > +        check_in_rootfs "${package}" || continue 
>
> This reads like you skip the download when you found package to _not_ 
> be "in_rootfs". 
>

Sorry that it is confusing. I will fix these to be more clear in the next 
series.
 

>
> Henning 
>
> >          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. 
>
>

[-- Attachment #1.2: Type: text/html, Size: 5311 bytes --]

[PATCH v5 06/13] deb-dl-dir: Factor out the mounting part

[Vijai Kumar K]

Factor out the mount and unmount section to separate function for
easy readability. No functional change intended.

Signed-off-by: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com>
---
 meta/classes/deb-dl-dir.bbclass | 33 +++++++++++++++++++++++----------
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/meta/classes/deb-dl-dir.bbclass b/meta/classes/deb-dl-dir.bbclass
index b3f4842..d53d4ae 100644
--- a/meta/classes/deb-dl-dir.bbclass
+++ b/meta/classes/deb-dl-dir.bbclass
@@ -14,6 +14,23 @@ check_in_rootfs() {
     [ -z "${output}" ] && return 1 || return 0
 }
 
+debsrc_do_mounts() {
+    sudo -s <<EOSUDO
+    mkdir -p "${1}/deb-src"
+    mountpoint -q "${1}/deb-src" || \
+    mount --bind "${DEBSRCDIR}" "${1}/deb-src"
+EOSUDO
+}
+
+debsrc_undo_mounts() {
+    sudo -s <<EOSUDO
+    mkdir -p "${1}/deb-src"
+    mountpoint -q "${1}/deb-src" && \
+    umount -l "${1}/deb-src"
+    rm -rf "${1}/deb-src"
+EOSUDO
+}
+
 debsrc_download() {
     export rootfs="$1"
     export rootfs_distro="$2"
@@ -21,11 +38,9 @@ debsrc_download() {
     ( 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
+
+    debsrc_do_mounts "${rootfs}"
+
     find "${rootfs}/var/cache/apt/archives/" -maxdepth 1 -type f -iname '*\.deb' | while read package; do
         check_in_rootfs "${package}" || continue
         local src="$( dpkg-deb --show --showformat '${Source}' "${package}" )"
@@ -49,11 +64,9 @@ EOSUDO
         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
+
+    debsrc_undo_mounts "${rootfs}"
+
     ) 9>"${DEBSRCDIR}/${rootfs_distro}.lock"
 }
 
-- 
2.17.1

[PATCH v5 07/13] deb-dl-dir: Fix skipping of removed files

[Vijai Kumar K]

Some packages are installed and then removed, like localepurge from
image-locales-extension.bbclass. Those information would not be
available in dpkg status file. Use dpkg log instead to see if the
package has been used on the target.

Signed-off-by: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com>
---
 meta/classes/deb-dl-dir.bbclass | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/meta/classes/deb-dl-dir.bbclass b/meta/classes/deb-dl-dir.bbclass
index d53d4ae..2567183 100644
--- a/meta/classes/deb-dl-dir.bbclass
+++ b/meta/classes/deb-dl-dir.bbclass
@@ -7,10 +7,13 @@ inherit repository
 
 check_in_rootfs() {
     local package="$( dpkg-deb --show --showformat '${Package}' "${1}" )"
-    local output="$( grep -hs "^Package: ${package}" \
-            "${IMAGE_ROOTFS}"/var/lib/dpkg/status \
-            "${BUILDCHROOT_HOST_DIR}"/var/lib/dpkg/status \
-            "${BUILDCHROOT_TARGET_DIR}"/var/lib/dpkg/status )"
+    local arch="$( dpkg-deb --show --showformat '${Architecture}' "${1}" )"
+    local version="$( dpkg-deb --show --showformat '${Version}' "${1}" )"
+    local output="$( grep -hs "status installed ${package}:${arch} ${version}" \
+            "${IMAGE_ROOTFS}"/var/log/dpkg.log \
+            "${BUILDCHROOT_HOST_DIR}"/var/log/dpkg.log \
+            "${BUILDCHROOT_TARGET_DIR}"/var/log/dpkg.log | head -1 )"
+
     [ -z "${output}" ] && return 1 || return 0
 }
 
-- 
2.17.1

[PATCH v5 08/13] repository: Add a sanity test to check missing sources

[Vijai Kumar K]

Add a sanity test routine to test if sources for all packages
are present in the repo.

Signed-off-by: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com>
---
 meta/classes/repository.bbclass            | 10 ++++++++++
 meta/recipes-devtools/base-apt/base-apt.bb |  2 ++
 2 files changed, 12 insertions(+)

diff --git a/meta/classes/repository.bbclass b/meta/classes/repository.bbclass
index c70138f..7b6e47b 100644
--- a/meta/classes/repository.bbclass
+++ b/meta/classes/repository.bbclass
@@ -96,3 +96,13 @@ repo_contains_package() {
     # no
     return 2
 }
+
+repo_sanity_test() {
+    local dir="$1"
+    local dbdir="$2"
+    local output="$( reprepro -s -b "${dir}" --dbdir "${dbdir}" sourcemissing )"
+    if [ -n "${output}" ]; then
+        bbwarn "One or more sources are missing in base-apt"
+        bbwarn "${output}"
+    fi
+}
diff --git a/meta/recipes-devtools/base-apt/base-apt.bb b/meta/recipes-devtools/base-apt/base-apt.bb
index da3e32e..8940ae8 100644
--- a/meta/recipes-devtools/base-apt/base-apt.bb
+++ b/meta/recipes-devtools/base-apt/base-apt.bb
@@ -66,6 +66,8 @@ repo() {
     fi
 
     populate_base_apt
+    repo_sanity_test "${REPO_BASE_DIR}"/"${BASE_DISTRO}" \
+        "${REPO_BASE_DB_DIR}"/"${BASE_DISTRO}"
 }
 
 python do_cache() {
-- 
2.17.1

[PATCH v5 09/13] base-apt: Introduce BASE_REPO_FEATURES

[Vijai Kumar K]

Eventhough we are collecting the debsrcs as part of postprocess,
it could not be considered a ROOTFS_FEATURE, instead a base-apt one.
Introduce BASE_REPO_FEATURES, to provide user with control to enable
or disable cache-deb-src. Disabled by default, since it is not required
for normal offline build to work.

Signed-off-by: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com>
---
 meta-isar/conf/local.conf.sample           | 4 ++++
 meta/classes/image.bbclass                 | 4 +++-
 meta/classes/repository.bbclass            | 9 +++++----
 meta/classes/rootfs.bbclass                | 1 -
 meta/recipes-devtools/base-apt/base-apt.bb | 1 +
 5 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/meta-isar/conf/local.conf.sample b/meta-isar/conf/local.conf.sample
index 274cdae..8389bac 100644
--- a/meta-isar/conf/local.conf.sample
+++ b/meta-isar/conf/local.conf.sample
@@ -176,6 +176,10 @@ IMAGE_INSTALL = "hello-isar example-raw example-module-${KERNEL_NAME} enable-fsc
 # NOTE: this works on build host >= stretch for armhf, arm64 and amd64 targets for now.
 ISAR_CROSS_COMPILE ?= "0"
 
+#
+# Uncomment this to enable caching of all source packages.
+# Without this feature, only sources of packages downloaded with apt:// are downloaded.
+#BASE_REPO_FEATURES ?= "cache-deb-src"
 #
 # Uncomment this to enable use of cached base repository
 #ISAR_USE_CACHED_BASE_REPO ?= "1"
diff --git a/meta/classes/image.bbclass b/meta/classes/image.bbclass
index fcaebd6..6131d6d 100644
--- a/meta/classes/image.bbclass
+++ b/meta/classes/image.bbclass
@@ -63,10 +63,12 @@ image_do_mounts() {
 }
 
 ROOTFSDIR = "${IMAGE_ROOTFS}"
-ROOTFS_FEATURES += "clean-package-cache generate-manifest cach-deb-src"
+ROOTFS_FEATURES += "clean-package-cache generate-manifest"
 ROOTFS_PACKAGES += "${IMAGE_PREINSTALL} ${IMAGE_INSTALL}"
 ROOTFS_MANIFEST_DEPLOY_DIR ?= "${DEPLOY_DIR_IMAGE}"
 
+ROOTFS_POSTPROCESS_COMMAND_prepend = "${@bb.utils.contains('BASE_REPO_FEATURES', 'cache-deb-src', 'cache_deb_src', '', d)} "
+
 inherit rootfs
 inherit image-sdk-extension
 inherit image-tools-extension
diff --git a/meta/classes/repository.bbclass b/meta/classes/repository.bbclass
index 7b6e47b..1f475dc 100644
--- a/meta/classes/repository.bbclass
+++ b/meta/classes/repository.bbclass
@@ -100,9 +100,10 @@ repo_contains_package() {
 repo_sanity_test() {
     local dir="$1"
     local dbdir="$2"
-    local output="$( reprepro -s -b "${dir}" --dbdir "${dbdir}" sourcemissing )"
-    if [ -n "${output}" ]; then
-        bbwarn "One or more sources are missing in base-apt"
-        bbwarn "${output}"
+    if [ "${@bb.utils.contains('BASE_REPO_FEATURES', 'cache-deb-src', 'yes', 'no', d)}" = "yes" ];then
+        local output="$( reprepro -s -b "${dir}" --dbdir "${dbdir}" sourcemissing )"
+        if [ -n "${output}" ]; then
+            bbfatal "One or more sources are missing in repo. ${output}"
+        fi
     fi
 }
diff --git a/meta/classes/rootfs.bbclass b/meta/classes/rootfs.bbclass
index ee57989..c00a8bf 100644
--- a/meta/classes/rootfs.bbclass
+++ b/meta/classes/rootfs.bbclass
@@ -185,7 +185,6 @@ 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}
diff --git a/meta/recipes-devtools/base-apt/base-apt.bb b/meta/recipes-devtools/base-apt/base-apt.bb
index 8940ae8..506a28f 100644
--- a/meta/recipes-devtools/base-apt/base-apt.bb
+++ b/meta/recipes-devtools/base-apt/base-apt.bb
@@ -10,6 +10,7 @@ SRC_URI = "file://distributions.in"
 
 BASE_REPO_KEY ?= ""
 KEYFILES ?= ""
+BASE_REPO_FEATURES ?= ""
 
 populate_base_apt() {
     find "${DEBDIR}"/"${DISTRO}" -name '*\.deb' | while read package; do
-- 
2.17.1

[PATCH v5 10/13] repository: Fix failures due to missing section

[Vijai Kumar K]

reprepro's includedsc failed for certain packages like makedev while
complaining about missing Section information. This information is
optional according to Debian[1]. Use a dummy value to avoid such
failures.

[1] https://wiki.debian.org/DebianRepository/Format

Signed-off-by: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com>
---
 meta/classes/repository.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/classes/repository.bbclass b/meta/classes/repository.bbclass
index 1f475dc..ac395db 100644
--- a/meta/classes/repository.bbclass
+++ b/meta/classes/repository.bbclass
@@ -42,7 +42,7 @@ repo_add_srcpackage() {
     if [ -n "${GNUPGHOME}" ]; then
         export GNUPGHOME="${GNUPGHOME}"
     fi
-    reprepro -b "${dir}" --dbdir "${dbdir}" -C main -P source \
+    reprepro -b "${dir}" --dbdir "${dbdir}" -C main -S - -P source \
         includedsc "${codename}" \
         "$@"
 }
-- 
2.17.1

[PATCH v5 11/13] scripts/ci_build.sh: Enable deb-src caching

[Vijai Kumar K]

Enable debsrc caching for CI builds.

Signed-off-by: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com>
---
 scripts/ci_build.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/ci_build.sh b/scripts/ci_build.sh
index adc403b..f832d7c 100755
--- a/scripts/ci_build.sh
+++ b/scripts/ci_build.sh
@@ -172,6 +172,7 @@ if [ -n "$REPRO_BUILD" ]; then
     sed -i -e 's/^BB_NO_NETWORK/#BB_NO_NETWORK/g' conf/local.conf
 fi
 
+sed -i -e 's/^#BASE_REPO_FEATURES ?= "cache-deb-src"/BASE_REPO_FEATURES ?= "cache-deb-src"/g' conf/local.conf
 # Start cross build for the defined set of configurations
 sed -i -e 's/ISAR_CROSS_COMPILE ?= "0"/ISAR_CROSS_COMPILE ?= "1"/g' conf/local.conf
 bitbake $BB_ARGS $CROSS_TARGETS_SET
-- 
2.17.1

[PATCH v5 12/13] rootfs: Fix possible overwrite of existing resolv.conf

[Vijai Kumar K]

There is a possiblilty that one of the packages installed in the
rootfs provides /etc/resolv.conf and we might accidentally remove
it. Fix it by taking a backup of any existing resolv conf files
and restoring it later. This is needed since we could not effectively
move caching before rootfs_install_clean_files since we need the
latest dpkg log.

Signed-off-by: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com>
---
 meta/classes/rootfs.bbclass | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/meta/classes/rootfs.bbclass b/meta/classes/rootfs.bbclass
index c00a8bf..5d2b893 100644
--- a/meta/classes/rootfs.bbclass
+++ b/meta/classes/rootfs.bbclass
@@ -186,10 +186,18 @@ python do_rootfs_install() {
 addtask rootfs_install before do_rootfs_postprocess after do_unpack
 
 cache_deb_src() {
+    if [ -e "${ROOTFSDIR}"/etc/resolv.conf ]; then
+        sudo mv "${ROOTFSDIR}"/etc/resolv.conf "${ROOTFSDIR}"/etc/resolv.conf.isar
+    fi
     rootfs_install_resolvconf
+
     deb_dl_dir_import ${ROOTFSDIR} ${ROOTFS_DISTRO}
     debsrc_download ${ROOTFSDIR} ${ROOTFS_DISTRO}
-    rootfs_install_clean_files
+
+    sudo rm -f "${ROOTFSDIR}"/etc/resolv.conf
+    if [ -e "${ROOTFSDIR}"/etc/resolv.conf.isar ]; then
+        sudo mv "${ROOTFSDIR}"/etc/resolv.conf.isar "${ROOTFSDIR}"/etc/resolv.conf
+    fi
 }
 
 ROOTFS_POSTPROCESS_COMMAND += "${@bb.utils.contains('ROOTFS_FEATURES', 'clean-package-cache', 'rootfs_postprocess_clean_package_cache', '', d)}"
-- 
2.17.1

[PATCH v5 13/13] rootfs: Handle failures when postprocess is rerun

[Vijai Kumar K]

The apt state information in var/lib/apt/lists is cleared as
part of postprocessing. This makes apt-get calls in cache_deb_src
fail when rerunning the postprocess task.

Since we cannot run apt-get update again to refresh the state
information, copy the apt state information from the initial
bootstrapped image.

Signed-off-by: Vijai Kumar K <Vijaikumar_Kanagarajan@mentor.com>
---
 meta/classes/rootfs.bbclass | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/meta/classes/rootfs.bbclass b/meta/classes/rootfs.bbclass
index 5d2b893..1e33f86 100644
--- a/meta/classes/rootfs.bbclass
+++ b/meta/classes/rootfs.bbclass
@@ -190,6 +190,10 @@ cache_deb_src() {
         sudo mv "${ROOTFSDIR}"/etc/resolv.conf "${ROOTFSDIR}"/etc/resolv.conf.isar
     fi
     rootfs_install_resolvconf
+    # Note: ISAR updates the apt state information(apt-get update) only once during bootstrap and
+    # relies on that through out the build. Copy that state information instead of apt-get update
+    # which generates a new state from upstream.
+    sudo cp -Trpn "${BOOTSTRAP_SRC}/var/lib/apt/lists/" "${ROOTFSDIR}/var/lib/apt/lists/"
 
     deb_dl_dir_import ${ROOTFSDIR} ${ROOTFS_DISTRO}
     debsrc_download ${ROOTFSDIR} ${ROOTFS_DISTRO}
-- 
2.17.1

Re: [PATCH v5 00/13] Deb-src caching implementation

[vijai kumar]

[-- Attachment #1.1: Type: text/plain, Size: 2685 bytes --]


On Friday, April 17, 2020 at 3:00:51 PM UTC+5:30, vijai kumar wrote:
>
> Changes in v5: 
>  The major changes in this series are as below, 
>     - (P2) Handle cases where HOST_DISTRO!=DISTRO. This avoids mixing 
> of debs from different distro which inturn helps the src caching logic. 
>     - (P5) Avoid downloading package from other builds. 
>     - (P8) Add a reprepro based sanity test to check if the repo contains 
> the sources for all the debs. 
>     - (P9) Introduce a new variable BASE_REPO_FEATURES which provides 
> means 
> to enable or disable various base-apt features. (cache-deb-src for now) 
>     - Some fixes in (P12 & P13) 
> Also, addressed some review comments from Henning. 
> Git Tree: https://github.com/vj-kumar/isar/tree/vijai/debsrc5
>


CI build is successful.

http://ci.isar-build.org:8080/job/isar_vkk_devel/56/console

 

>
> Changes in v4: 
>  - Use <source package>=<version> format instead of just using 
> <packagename> 
> to download the right version of source package. 
>
> Changes in v3: 
>  - Take care of non-existent downloads/deb-src directory. 
>
> Changes in v2: 
>  - Introduced additional patch to cache deb src 
>  - Rebased on top of henning/staging4 tree 
>
>
> Vijai Kumar K (13): 
>   rootfs: Make rootfs finalize a separate task 
>   deb-dl-dir: Cache host distro debs separately 
>   meta: cache deb srcs as part of postprocessing 
>   deb-dl-dir: Make debsrc_download faster 
>   deb-dl-dir: Download files only belonging to the current image 
>   deb-dl-dir: Factor out the mounting part 
>   deb-dl-dir: Fix skipping of removed files 
>   repository: Add a sanity test to check missing sources 
>   base-apt: Introduce BASE_REPO_FEATURES 
>   repository: Fix failures due to missing section 
>   scripts/ci_build.sh: Enable deb-src caching 
>   rootfs: Fix possible overwrite of existing resolv.conf 
>   rootfs: Handle failures when postprocess is rerun 
>
>  meta-isar/conf/local.conf.sample              |  4 ++ 
>  meta/classes/deb-dl-dir.bbclass               | 72 ++++++++++++++++++- 
>  meta/classes/dpkg-gbp.bbclass                 |  8 ++- 
>  meta/classes/dpkg.bbclass                     |  8 ++- 
>  meta/classes/image-tools-extension.bbclass    |  4 +- 
>  meta/classes/image.bbclass                    | 43 ++++++++++- 
>  meta/classes/repository.bbclass               | 13 +++- 
>  meta/classes/rootfs.bbclass                   | 62 ++++++---------- 
>  .../isar-bootstrap/isar-bootstrap.inc         | 12 +++- 
>  meta/recipes-devtools/base-apt/base-apt.bb    |  3 + 
>  scripts/ci_build.sh                           |  1 + 
>  11 files changed, 177 insertions(+), 53 deletions(-) 
>
> -- 
> 2.17.1 
>
>
>

[-- Attachment #1.2: Type: text/html, Size: 4345 bytes --]

Re: [PATCH v5 00/13] Deb-src caching implementation

[Henning Schild]

Had to stop at p5 and will look at the others later.

Henning

On Fri, 17 Apr 2020 15:00:27 +0530
Vijai Kumar K <vijaikumar.kanagarajan@gmail.com> wrote:

> Changes in v5:
>  The major changes in this series are as below,
>     - (P2) Handle cases where HOST_DISTRO!=DISTRO. This avoids mixing
> of debs from different distro which inturn helps the src caching
> logic.
>     - (P5) Avoid downloading package from other builds.
>     - (P8) Add a reprepro based sanity test to check if the repo
> contains the sources for all the debs.
>     - (P9) Introduce a new variable BASE_REPO_FEATURES which provides
> means to enable or disable various base-apt features. (cache-deb-src
> for now)
>     - Some fixes in (P12 & P13)
> Also, addressed some review comments from Henning.
> Git Tree: https://github.com/vj-kumar/isar/tree/vijai/debsrc5
> 
> Changes in v4:
>  - Use <source package>=<version> format instead of just using
> <packagename> to download the right version of source package.
> 
> Changes in v3:
>  - Take care of non-existent downloads/deb-src directory.
> 
> Changes in v2:
>  - Introduced additional patch to cache deb src
>  - Rebased on top of henning/staging4 tree
> 
> 
> Vijai Kumar K (13):
>   rootfs: Make rootfs finalize a separate task
>   deb-dl-dir: Cache host distro debs separately
>   meta: cache deb srcs as part of postprocessing
>   deb-dl-dir: Make debsrc_download faster
>   deb-dl-dir: Download files only belonging to the current image
>   deb-dl-dir: Factor out the mounting part
>   deb-dl-dir: Fix skipping of removed files
>   repository: Add a sanity test to check missing sources
>   base-apt: Introduce BASE_REPO_FEATURES
>   repository: Fix failures due to missing section
>   scripts/ci_build.sh: Enable deb-src caching
>   rootfs: Fix possible overwrite of existing resolv.conf
>   rootfs: Handle failures when postprocess is rerun
> 
>  meta-isar/conf/local.conf.sample              |  4 ++
>  meta/classes/deb-dl-dir.bbclass               | 72
> ++++++++++++++++++- meta/classes/dpkg-gbp.bbclass                 |
> 8 ++- meta/classes/dpkg.bbclass                     |  8 ++-
>  meta/classes/image-tools-extension.bbclass    |  4 +-
>  meta/classes/image.bbclass                    | 43 ++++++++++-
>  meta/classes/repository.bbclass               | 13 +++-
>  meta/classes/rootfs.bbclass                   | 62 ++++++----------
>  .../isar-bootstrap/isar-bootstrap.inc         | 12 +++-
>  meta/recipes-devtools/base-apt/base-apt.bb    |  3 +
>  scripts/ci_build.sh                           |  1 +
>  11 files changed, 177 insertions(+), 53 deletions(-)
>