[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

[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_...@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

[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_...@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

[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_...@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

[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_...@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

[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_...@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

[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_...@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

[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_...@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

[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_...@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

[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_...@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

[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_...@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

[Vijai Kumar K]

Enable debsrc caching for CI builds.

Signed-off-by: Vijai Kumar K <Vijaikumar_...@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

[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_...@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

[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_...@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

[vijai kumar]

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

[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

[Henning Schild]

On Fri, 17 Apr 2020 15:00:30 +0530

Vijai Kumar K <vijaikumar....@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_...@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

[Henning Schild]

On Fri, 17 Apr 2020 15:00:31 +0530

Vijai Kumar K <vijaikumar....@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_...@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

[Henning Schild]

On Fri, 17 Apr 2020 15:00:32 +0530

Vijai Kumar K <vijaikumar....@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_...@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

[Henning Schild]

On Fri, 17 Apr 2020 15:00:30 +0530

Vijai Kumar K <vijaikumar....@gmail.com> wrote:

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

[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....@gmail.com> wrote:

[vijai kumar]

On Wednesday, April 22, 2020 at 12:36:42 PM UTC+5:30, Henning Schild wrote:

On Fri, 17 Apr 2020 15:00:30 +0530
Vijai Kumar K <vijaikumar...@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_...@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. 

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

[vijai kumar]

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

[vijai kumar]

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

[vijai kumar]

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 Schild]

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

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 Schild]

Am Wed, 22 Apr 2020 03:11:11 -0700 (PDT)
schrieb vijai kumar <vijaikumar....@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:>> ---

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

[vijai kumar]

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

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.

I did an offline build(with BB_NO_NETWORK set) with this series and it seems

to work fine.

Thanks,

Vijai Kumar K

[vijai kumar]

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. 

[vijai kumar]

Hi Henning,

Did you get a chance to review the rest of the series?

Thanks,
Vijai Kumar K

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

[vijaikumar....@gmail.com]

Ping

[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

[vijaikumar....@gmail.com]

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.