| Message ID | 1fe26da1-b3db-4b06-95ff-4b7996d318ee@siemens.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [1/2] dpkg-source: Fix source deployment | expand |
Kiszka, Jan, Sonntag, 5. Mai 2024 22:33: > Avoid building the source package multiple times, possibly even > inconsistently. This is achieved by delegating this task to to the base > package and installing the source package from isar-apt in the native > and compat package variants. Hi Jan, I thought about changes along those lines a while back, and had two problems that made this difficult for me: - This assumes that the source packages are identical for different build variants. That is how it should be (there should be only one source, possibly even across distro releases and architectures), but Isar gives developers the tools to break that rule (SRC_URI:append:xy = "some.patch"), and they will use those tools! So we'd need at least documentation warning about this, or probably better some checks that restrict how SRC_URI can be constructed. - In dpkg_runbuild (dpkg.bbclass), the code that finds DEB_SOURCE_NAME assumes extracted sources (with a debian/changelog). Looking at your patch, I suspect this only works for you because you still have a dependency on do_fetch, even for the compat/native case where you probably want to only depend on the fetch_common_source job. To me it seems that we currently have an almost-but-not-100%-complete decoupling of the build into source generation and binary package build. I'd propose that we first complete that split (solving the two problems I mentioned). This would then also allow sstate caching of the common source packages, and it should make it possible for all build variants to use the same code path, without the need for the is_native_or_compat branch you have in your patch. Adriaan > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > meta/classes/dpkg-source.bbclass | 44 ++++++++++++++++++++++++++++++-- > 1 file changed, 42 insertions(+), 2 deletions(-) > > diff --git a/meta/classes/dpkg-source.bbclass b/meta/classes/dpkg- > source.bbclass > index 005eafbe..7161641f 100644 > --- a/meta/classes/dpkg-source.bbclass > +++ b/meta/classes/dpkg-source.bbclass > @@ -13,7 +13,7 @@ do_dpkg_source() { > find ${WORKDIR} -name "${DEB_SOURCE_NAME}*.dsc" -maxdepth 1 -delete > sh -c "cd ${WORKDIR}; dpkg-source ${DPKG_SOURCE_EXTRA_ARGS} -b ${PPS}" > } > -addtask dpkg_source after do_prepare_build before do_dpkg_build > +addtask dpkg_source after do_prepare_build > > do_deploy_source[depends] += "isar-apt:do_cache_config" > do_deploy_source[lockfiles] = "${REPO_ISAR_DIR}/isar.lock" > @@ -28,4 +28,44 @@ do_deploy_source() { > "${package}" > done > } > -addtask deploy_source after do_dpkg_source before do_dpkg_build > +addtask deploy_source after do_dpkg_source > + > +do_dpkg_build[depends] += "${BPN}:do_deploy_source" > + > +SCHROOT_MOUNTS = "${WORKDIR}:/work ${REPO_ISAR_DIR}/${DISTRO}:/isar-apt" > + > +do_fetch_common_source[depends] += "${BPN}:do_deploy_source" > +do_fetch_common_source[network] = "${TASK_USE_SUDO}" > +do_fetch_common_source() { > + schroot_create_configs > + insert_mounts > + > + session_id=$(schroot -q -b -c ${SBUILD_CHROOT}) > + echo "Started session: ${session_id}" > + > + schroot_cleanup() { > + schroot -q -f -e -c ${session_id} > /dev/null 2>&1 > + remove_mounts > /dev/null 2>&1 > + schroot_delete_configs > + } > + trap 'exit 1' INT HUP QUIT TERM ALRM USR1 > + trap 'schroot_cleanup' EXIT > + > + schroot -r -c ${session_id} -d / -u root -- \ > + apt-get update -o Dir::Etc::SourceList="sources.list.d/isar- > apt.list" -o Dir::Etc::SourceParts="-" -o APT::Get::List-Cleanup="0" > + schroot -r -c ${session_id} -d / -- \ > + sh -c ' > + cd /work > + apt-get -y --download-only --only-source -o Acquire::Source- > Symlinks="false" source ${BPN}' > + > + schroot -e -c ${session_id} > + remove_mounts > + schroot_delete_configs > +} > +addtask fetch_common_source after do_unpack > + > +def is_native_or_compat(d): > + overrides = d.getVar('OVERRIDES').split(':') > + return 'class-native' in overrides or 'class-compat' in overrides > + > +do_dpkg_build[depends] += "${@'${PN}:do_fetch_common_source' if > is_native_or_compat(d) else ''}" > -- > 2.35.3
On 06.05.24 08:29, Schmidt, Adriaan (T CED EDC-DE) wrote: > Kiszka, Jan, Sonntag, 5. Mai 2024 22:33: >> Avoid building the source package multiple times, possibly even >> inconsistently. This is achieved by delegating this task to to the base >> package and installing the source package from isar-apt in the native >> and compat package variants. > > Hi Jan, > > I thought about changes along those lines a while back, and had two problems > that made this difficult for me: > > - This assumes that the source packages are identical for different build > variants. That is how it should be (there should be only one source, possibly > even across distro releases and architectures), but Isar gives developers > the tools to break that rule (SRC_URI:append:xy = "some.patch"), and > they will use those tools! So we'd need at least documentation warning > about this, or probably better some checks that restrict how SRC_URI > can be constructed. If developers do the wrong thing (like in this patch), they get what they deserve. My patch prevents that this silently sneaks into production. > - In dpkg_runbuild (dpkg.bbclass), the code that finds DEB_SOURCE_NAME > assumes extracted sources (with a debian/changelog). Looking at your > patch, I suspect this only works for you because you still have a > dependency on do_fetch, even for the compat/native case where > you probably want to only depend on the fetch_common_source job. To my understanding, installing the source package ensures this. Jan
On Sun, 2024-05-05 at 22:32 +0200, Jan Kiszka wrote: > From: Jan Kiszka <jan.kiszka@siemens.com> > > Avoid building the source package multiple times, possibly even > inconsistently. This is achieved by delegating this task to to the > base > package and installing the source package from isar-apt in the native > and compat package variants. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > meta/classes/dpkg-source.bbclass | 44 > ++++++++++++++++++++++++++++++-- > 1 file changed, 42 insertions(+), 2 deletions(-) > > diff --git a/meta/classes/dpkg-source.bbclass b/meta/classes/dpkg- > source.bbclass > index 005eafbe..7161641f 100644 > --- a/meta/classes/dpkg-source.bbclass > +++ b/meta/classes/dpkg-source.bbclass > @@ -13,7 +13,7 @@ do_dpkg_source() { > find ${WORKDIR} -name "${DEB_SOURCE_NAME}*.dsc" -maxdepth 1 - > delete > sh -c "cd ${WORKDIR}; dpkg-source ${DPKG_SOURCE_EXTRA_ARGS} -b > ${PPS}" > } > -addtask dpkg_source after do_prepare_build before do_dpkg_build > +addtask dpkg_source after do_prepare_build > > do_deploy_source[depends] += "isar-apt:do_cache_config" > do_deploy_source[lockfiles] = "${REPO_ISAR_DIR}/isar.lock" > @@ -28,4 +28,44 @@ do_deploy_source() { > "${package}" > done > } > -addtask deploy_source after do_dpkg_source before do_dpkg_build > +addtask deploy_source after do_dpkg_source > + > +do_dpkg_build[depends] += "${BPN}:do_deploy_source" > + > +SCHROOT_MOUNTS = "${WORKDIR}:/work ${REPO_ISAR_DIR}/${DISTRO}:/isar- > apt" > + > +do_fetch_common_source[depends] += "${BPN}:do_deploy_source" > +do_fetch_common_source[network] = "${TASK_USE_SUDO}" > +do_fetch_common_source() { > + schroot_create_configs > + insert_mounts > + > + session_id=$(schroot -q -b -c ${SBUILD_CHROOT}) > + echo "Started session: ${session_id}" > + > + schroot_cleanup() { > + schroot -q -f -e -c ${session_id} > /dev/null 2>&1 > + remove_mounts > /dev/null 2>&1 > + schroot_delete_configs > + } > + trap 'exit 1' INT HUP QUIT TERM ALRM USR1 > + trap 'schroot_cleanup' EXIT > + > + schroot -r -c ${session_id} -d / -u root -- \ > + apt-get update -o Dir::Etc::SourceList="sources.list.d/isar- > apt.list" -o Dir::Etc::SourceParts="-" -o APT::Get::List-Cleanup="0" > + schroot -r -c ${session_id} -d / -- \ > + sh -c ' > + cd /work > + apt-get -y --download-only --only-source -o > Acquire::Source-Symlinks="false" source ${BPN}' > + > + schroot -e -c ${session_id} > + remove_mounts > + schroot_delete_configs > +} > +addtask fetch_common_source after do_unpack > + > +def is_native_or_compat(d): > + overrides = d.getVar('OVERRIDES').split(':') > + return 'class-native' in overrides or 'class-compat' in > overrides > + > +do_dpkg_build[depends] += "${@'${PN}:do_fetch_common_source' if > is_native_or_compat(d) else ''}" Thanks. For the linux-custom build the linux-....tar.gz sources are created not only for linux-native but also for linux-kbuildtarget... Stefan
On 06.05.24 10:11, Koch, Stefan (DI PA DCP R&D 3) wrote: > On Sun, 2024-05-05 at 22:32 +0200, Jan Kiszka wrote: >> From: Jan Kiszka <jan.kiszka@siemens.com> >> >> Avoid building the source package multiple times, possibly even >> inconsistently. This is achieved by delegating this task to to the >> base >> package and installing the source package from isar-apt in the native >> and compat package variants. >> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >> --- >> meta/classes/dpkg-source.bbclass | 44 >> ++++++++++++++++++++++++++++++-- >> 1 file changed, 42 insertions(+), 2 deletions(-) >> >> diff --git a/meta/classes/dpkg-source.bbclass b/meta/classes/dpkg- >> source.bbclass >> index 005eafbe..7161641f 100644 >> --- a/meta/classes/dpkg-source.bbclass >> +++ b/meta/classes/dpkg-source.bbclass >> @@ -13,7 +13,7 @@ do_dpkg_source() { >> find ${WORKDIR} -name "${DEB_SOURCE_NAME}*.dsc" -maxdepth 1 - >> delete >> sh -c "cd ${WORKDIR}; dpkg-source ${DPKG_SOURCE_EXTRA_ARGS} -b >> ${PPS}" >> } >> -addtask dpkg_source after do_prepare_build before do_dpkg_build >> +addtask dpkg_source after do_prepare_build >> >> do_deploy_source[depends] += "isar-apt:do_cache_config" >> do_deploy_source[lockfiles] = "${REPO_ISAR_DIR}/isar.lock" >> @@ -28,4 +28,44 @@ do_deploy_source() { >> "${package}" >> done >> } >> -addtask deploy_source after do_dpkg_source before do_dpkg_build >> +addtask deploy_source after do_dpkg_source >> + >> +do_dpkg_build[depends] += "${BPN}:do_deploy_source" >> + >> +SCHROOT_MOUNTS = "${WORKDIR}:/work ${REPO_ISAR_DIR}/${DISTRO}:/isar- >> apt" >> + >> +do_fetch_common_source[depends] += "${BPN}:do_deploy_source" >> +do_fetch_common_source[network] = "${TASK_USE_SUDO}" >> +do_fetch_common_source() { >> + schroot_create_configs >> + insert_mounts >> + >> + session_id=$(schroot -q -b -c ${SBUILD_CHROOT}) >> + echo "Started session: ${session_id}" >> + >> + schroot_cleanup() { >> + schroot -q -f -e -c ${session_id} > /dev/null 2>&1 >> + remove_mounts > /dev/null 2>&1 >> + schroot_delete_configs >> + } >> + trap 'exit 1' INT HUP QUIT TERM ALRM USR1 >> + trap 'schroot_cleanup' EXIT >> + >> + schroot -r -c ${session_id} -d / -u root -- \ >> + apt-get update -o Dir::Etc::SourceList="sources.list.d/isar- >> apt.list" -o Dir::Etc::SourceParts="-" -o APT::Get::List-Cleanup="0" >> + schroot -r -c ${session_id} -d / -- \ >> + sh -c ' >> + cd /work >> + apt-get -y --download-only --only-source -o >> Acquire::Source-Symlinks="false" source ${BPN}' >> + >> + schroot -e -c ${session_id} >> + remove_mounts >> + schroot_delete_configs >> +} >> +addtask fetch_common_source after do_unpack >> + >> +def is_native_or_compat(d): >> + overrides = d.getVar('OVERRIDES').split(':') >> + return 'class-native' in overrides or 'class-compat' in >> overrides >> + >> +do_dpkg_build[depends] += "${@'${PN}:do_fetch_common_source' if >> is_native_or_compat(d) else ''}" > Thanks. For the linux-custom build the linux-....tar.gz sources are > created not only for linux-native but also for linux-kbuildtarget... > That target is not stressed in upstream, it looks like - recipe for bitrotting... Jan
05/05/2024 23:32, 'Jan Kiszka' via isar-users wrote: > From: Jan Kiszka <jan.kiszka@siemens.com> > > Avoid building the source package multiple times, possibly even > inconsistently. This is achieved by delegating this task to to the base > package and installing the source package from isar-apt in the native > and compat package variants. > > Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> > --- > meta/classes/dpkg-source.bbclass | 44 ++++++++++++++++++++++++++++++-- > 1 file changed, 42 insertions(+), 2 deletions(-) > > diff --git a/meta/classes/dpkg-source.bbclass b/meta/classes/dpkg-source.bbclass > index 005eafbe..7161641f 100644 > --- a/meta/classes/dpkg-source.bbclass > +++ b/meta/classes/dpkg-source.bbclass > @@ -13,7 +13,7 @@ do_dpkg_source() { > find ${WORKDIR} -name "${DEB_SOURCE_NAME}*.dsc" -maxdepth 1 -delete > sh -c "cd ${WORKDIR}; dpkg-source ${DPKG_SOURCE_EXTRA_ARGS} -b ${PPS}" > } > -addtask dpkg_source after do_prepare_build before do_dpkg_build > +addtask dpkg_source after do_prepare_build > > do_deploy_source[depends] += "isar-apt:do_cache_config" > do_deploy_source[lockfiles] = "${REPO_ISAR_DIR}/isar.lock" > @@ -28,4 +28,44 @@ do_deploy_source() { > "${package}" > done > } > -addtask deploy_source after do_dpkg_source before do_dpkg_build > +addtask deploy_source after do_dpkg_source > + > +do_dpkg_build[depends] += "${BPN}:do_deploy_source" > + > +SCHROOT_MOUNTS = "${WORKDIR}:/work ${REPO_ISAR_DIR}/${DISTRO}:/isar-apt" > + > +do_fetch_common_source[depends] += "${BPN}:do_deploy_source" As do_fetch_common_source is the first task which use schroot it should also have ${SCHROOT_DEP} in depends flag. Simple test case to reproduce the issue with missing schroot: build mc:qemuarm64-bullseye:libhello-compat (with ISAR_ENABLE_COMPAT_ARCH enabled) > +do_fetch_common_source[network] = "${TASK_USE_SUDO}" > +do_fetch_common_source() { > + schroot_create_configs > + insert_mounts > + > + session_id=$(schroot -q -b -c ${SBUILD_CHROOT}) > + echo "Started session: ${session_id}" > + > + schroot_cleanup() { > + schroot -q -f -e -c ${session_id} > /dev/null 2>&1 > + remove_mounts > /dev/null 2>&1 > + schroot_delete_configs > + } > + trap 'exit 1' INT HUP QUIT TERM ALRM USR1 > + trap 'schroot_cleanup' EXIT > + > + schroot -r -c ${session_id} -d / -u root -- \ > + apt-get update -o Dir::Etc::SourceList="sources.list.d/isar-apt.list" -o Dir::Etc::SourceParts="-" -o APT::Get::List-Cleanup="0" > + schroot -r -c ${session_id} -d / -- \ > + sh -c ' > + cd /work > + apt-get -y --download-only --only-source -o Acquire::Source-Symlinks="false" source ${BPN}' > + > + schroot -e -c ${session_id} > + remove_mounts > + schroot_delete_configs > +} > +addtask fetch_common_source after do_unpack > + > +def is_native_or_compat(d): > + overrides = d.getVar('OVERRIDES').split(':') > + return 'class-native' in overrides or 'class-compat' in overrides > + > +do_dpkg_build[depends] += "${@'${PN}:do_fetch_common_source' if is_native_or_compat(d) else ''}"
On 13.05.24 14:04, Anton Mikanovich wrote: > 05/05/2024 23:32, 'Jan Kiszka' via isar-users wrote: >> From: Jan Kiszka <jan.kiszka@siemens.com> >> >> Avoid building the source package multiple times, possibly even >> inconsistently. This is achieved by delegating this task to to the base >> package and installing the source package from isar-apt in the native >> and compat package variants. >> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com> >> --- >> meta/classes/dpkg-source.bbclass | 44 ++++++++++++++++++++++++++++++-- >> 1 file changed, 42 insertions(+), 2 deletions(-) >> >> diff --git a/meta/classes/dpkg-source.bbclass >> b/meta/classes/dpkg-source.bbclass >> index 005eafbe..7161641f 100644 >> --- a/meta/classes/dpkg-source.bbclass >> +++ b/meta/classes/dpkg-source.bbclass >> @@ -13,7 +13,7 @@ do_dpkg_source() { >> find ${WORKDIR} -name "${DEB_SOURCE_NAME}*.dsc" -maxdepth 1 -delete >> sh -c "cd ${WORKDIR}; dpkg-source ${DPKG_SOURCE_EXTRA_ARGS} -b >> ${PPS}" >> } >> -addtask dpkg_source after do_prepare_build before do_dpkg_build >> +addtask dpkg_source after do_prepare_build >> do_deploy_source[depends] += "isar-apt:do_cache_config" >> do_deploy_source[lockfiles] = "${REPO_ISAR_DIR}/isar.lock" >> @@ -28,4 +28,44 @@ do_deploy_source() { >> "${package}" >> done >> } >> -addtask deploy_source after do_dpkg_source before do_dpkg_build >> +addtask deploy_source after do_dpkg_source >> + >> +do_dpkg_build[depends] += "${BPN}:do_deploy_source" >> + >> +SCHROOT_MOUNTS = "${WORKDIR}:/work ${REPO_ISAR_DIR}/${DISTRO}:/isar-apt" >> + >> +do_fetch_common_source[depends] += "${BPN}:do_deploy_source" > As do_fetch_common_source is the first task which use schroot it should > also > have ${SCHROOT_DEP} in depends flag. Yes, locally fixed already. There was another bug as well, forgot right now. Will send a new series once Adriaan sent his v4 of the kbuild rework. Jan > Simple test case to reproduce the issue with missing schroot: build > mc:qemuarm64-bullseye:libhello-compat (with ISAR_ENABLE_COMPAT_ARCH > enabled) >> +do_fetch_common_source[network] = "${TASK_USE_SUDO}" >> +do_fetch_common_source() { >> + schroot_create_configs >> + insert_mounts >> + >> + session_id=$(schroot -q -b -c ${SBUILD_CHROOT}) >> + echo "Started session: ${session_id}" >> + >> + schroot_cleanup() { >> + schroot -q -f -e -c ${session_id} > /dev/null 2>&1 >> + remove_mounts > /dev/null 2>&1 >> + schroot_delete_configs >> + } >> + trap 'exit 1' INT HUP QUIT TERM ALRM USR1 >> + trap 'schroot_cleanup' EXIT >> + >> + schroot -r -c ${session_id} -d / -u root -- \ >> + apt-get update -o >> Dir::Etc::SourceList="sources.list.d/isar-apt.list" -o >> Dir::Etc::SourceParts="-" -o APT::Get::List-Cleanup="0" >> + schroot -r -c ${session_id} -d / -- \ >> + sh -c ' >> + cd /work >> + apt-get -y --download-only --only-source -o >> Acquire::Source-Symlinks="false" source ${BPN}' >> + >> + schroot -e -c ${session_id} >> + remove_mounts >> + schroot_delete_configs >> +} >> +addtask fetch_common_source after do_unpack >> + >> +def is_native_or_compat(d): >> + overrides = d.getVar('OVERRIDES').split(':') >> + return 'class-native' in overrides or 'class-compat' in overrides >> + >> +do_dpkg_build[depends] += "${@'${PN}:do_fetch_common_source' if >> is_native_or_compat(d) else ''}" > >
diff --git a/meta/classes/dpkg-source.bbclass b/meta/classes/dpkg-source.bbclass index 005eafbe..7161641f 100644 --- a/meta/classes/dpkg-source.bbclass +++ b/meta/classes/dpkg-source.bbclass @@ -13,7 +13,7 @@ do_dpkg_source() { find ${WORKDIR} -name "${DEB_SOURCE_NAME}*.dsc" -maxdepth 1 -delete sh -c "cd ${WORKDIR}; dpkg-source ${DPKG_SOURCE_EXTRA_ARGS} -b ${PPS}" } -addtask dpkg_source after do_prepare_build before do_dpkg_build +addtask dpkg_source after do_prepare_build do_deploy_source[depends] += "isar-apt:do_cache_config" do_deploy_source[lockfiles] = "${REPO_ISAR_DIR}/isar.lock" @@ -28,4 +28,44 @@ do_deploy_source() { "${package}" done } -addtask deploy_source after do_dpkg_source before do_dpkg_build +addtask deploy_source after do_dpkg_source + +do_dpkg_build[depends] += "${BPN}:do_deploy_source" + +SCHROOT_MOUNTS = "${WORKDIR}:/work ${REPO_ISAR_DIR}/${DISTRO}:/isar-apt" + +do_fetch_common_source[depends] += "${BPN}:do_deploy_source" +do_fetch_common_source[network] = "${TASK_USE_SUDO}" +do_fetch_common_source() { + schroot_create_configs + insert_mounts + + session_id=$(schroot -q -b -c ${SBUILD_CHROOT}) + echo "Started session: ${session_id}" + + schroot_cleanup() { + schroot -q -f -e -c ${session_id} > /dev/null 2>&1 + remove_mounts > /dev/null 2>&1 + schroot_delete_configs + } + trap 'exit 1' INT HUP QUIT TERM ALRM USR1 + trap 'schroot_cleanup' EXIT + + schroot -r -c ${session_id} -d / -u root -- \ + apt-get update -o Dir::Etc::SourceList="sources.list.d/isar-apt.list" -o Dir::Etc::SourceParts="-" -o APT::Get::List-Cleanup="0" + schroot -r -c ${session_id} -d / -- \ + sh -c ' + cd /work + apt-get -y --download-only --only-source -o Acquire::Source-Symlinks="false" source ${BPN}' + + schroot -e -c ${session_id} + remove_mounts + schroot_delete_configs +} +addtask fetch_common_source after do_unpack + +def is_native_or_compat(d): + overrides = d.getVar('OVERRIDES').split(':') + return 'class-native' in overrides or 'class-compat' in overrides + +do_dpkg_build[depends] += "${@'${PN}:do_fetch_common_source' if is_native_or_compat(d) else ''}"