[1/2] dpkg-source: Fix source deployment

Message ID d6091740-4f66-4431-b03d-d93471e00fd6@siemens.com
State Superseded, archived
Headers show
Series [1/2] dpkg-source: Fix source deployment | expand

Commit Message

Jan Kiszka May 5, 2024, 8:32 p.m. UTC
From: Jan Kiszka <jan.kiszka@siemens.com>

This failed if S was not a direct subdir of WORKDIR. Align it with
do_dpkg_source.

Furthermore, move -maxdepth before -name to prevent warnings about a
global option specified after a positional one.

Fixes: 38b832ad8248 ("meta: Implement two stage build")
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 meta/classes/dpkg-source.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jan Kiszka May 10, 2024, 5:32 p.m. UTC | #1
On 05.05.24 22:32, 'Jan Kiszka' via isar-users wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> This failed if S was not a direct subdir of WORKDIR. Align it with
> do_dpkg_source.
> 
> Furthermore, move -maxdepth before -name to prevent warnings about a
> global option specified after a positional one.
> 
> Fixes: 38b832ad8248 ("meta: Implement two stage build")
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  meta/classes/dpkg-source.bbclass | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meta/classes/dpkg-source.bbclass b/meta/classes/dpkg-source.bbclass
> index 7fd5d2ed..005eafbe 100644
> --- a/meta/classes/dpkg-source.bbclass
> +++ b/meta/classes/dpkg-source.bbclass
> @@ -21,7 +21,7 @@ do_deploy_source[dirs] = "${S}"
>  do_deploy_source() {
>      repo_del_srcpackage "${REPO_ISAR_DIR}"/"${DISTRO}" \
>          "${REPO_ISAR_DB_DIR}"/"${DISTRO}" "${DEBDISTRONAME}" "${BPN}"
> -    find "${S}/../" -name '*\.dsc' -maxdepth 1 | while read package; do
> +    find "${WORKDIR}" -maxdepth 1 -name '*\.dsc' | while read package; do
>          repo_add_srcpackage "${REPO_ISAR_DIR}"/"${DISTRO}" \
>              "${REPO_ISAR_DB_DIR}"/"${DISTRO}" \
>              "${DEBDISTRONAME}" \

The longer I look at this... Why is this a loop over potentially
multiple dsc files? We are expecting a single source package, and that
is assumed to be called ${BPN} (that assumption for cleaning may be
wrong with external source packages, though). So there should be also
only a single match for *_*.dsc, no?

I'm actually trying the clean the mess we have in dpkg_runbuild where we
are reading debian/changlog to get the dsc name. When not building the
source packages ourselves, we would have to unpack the sources in order
get the changelog in order to find out the dsc name that is needed to
unpack the source - this is nonsense.

Jan
Jan Kiszka May 10, 2024, 5:55 p.m. UTC | #2
On 10.05.24 19:32, Jan Kiszka wrote:
> On 05.05.24 22:32, 'Jan Kiszka' via isar-users wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> This failed if S was not a direct subdir of WORKDIR. Align it with
>> do_dpkg_source.
>>
>> Furthermore, move -maxdepth before -name to prevent warnings about a
>> global option specified after a positional one.
>>
>> Fixes: 38b832ad8248 ("meta: Implement two stage build")
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  meta/classes/dpkg-source.bbclass | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/meta/classes/dpkg-source.bbclass b/meta/classes/dpkg-source.bbclass
>> index 7fd5d2ed..005eafbe 100644
>> --- a/meta/classes/dpkg-source.bbclass
>> +++ b/meta/classes/dpkg-source.bbclass
>> @@ -21,7 +21,7 @@ do_deploy_source[dirs] = "${S}"
>>  do_deploy_source() {
>>      repo_del_srcpackage "${REPO_ISAR_DIR}"/"${DISTRO}" \
>>          "${REPO_ISAR_DB_DIR}"/"${DISTRO}" "${DEBDISTRONAME}" "${BPN}"
>> -    find "${S}/../" -name '*\.dsc' -maxdepth 1 | while read package; do
>> +    find "${WORKDIR}" -maxdepth 1 -name '*\.dsc' | while read package; do
>>          repo_add_srcpackage "${REPO_ISAR_DIR}"/"${DISTRO}" \
>>              "${REPO_ISAR_DB_DIR}"/"${DISTRO}" \
>>              "${DEBDISTRONAME}" \
> 
> The longer I look at this... Why is this a loop over potentially
> multiple dsc files? We are expecting a single source package, and that
> is assumed to be called ${BPN} (that assumption for cleaning may be
> wrong with external source packages, though). So there should be also
> only a single match for *_*.dsc, no?
> 

Ok, there is a case where there could be multiple dsc files: If someone
fetches apt://foo in order to build bar.deb. Then we will not only
deploy the sources of bar to isar-apt but also those of the dependency
foo - ouch.

We need to figure out the source name of the target package differently.
Not so easy...

Jan

> I'm actually trying the clean the mess we have in dpkg_runbuild where we
> are reading debian/changlog to get the dsc name. When not building the
> source packages ourselves, we would have to unpack the sources in order
> get the changelog in order to find out the dsc name that is needed to
> unpack the source - this is nonsense.
> 
> Jan
>
Jan Kiszka May 13, 2024, 6:56 a.m. UTC | #3
On 10.05.24 19:32, Jan Kiszka wrote:
> On 05.05.24 22:32, 'Jan Kiszka' via isar-users wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> This failed if S was not a direct subdir of WORKDIR. Align it with
>> do_dpkg_source.
>>
>> Furthermore, move -maxdepth before -name to prevent warnings about a
>> global option specified after a positional one.
>>
>> Fixes: 38b832ad8248 ("meta: Implement two stage build")
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  meta/classes/dpkg-source.bbclass | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/meta/classes/dpkg-source.bbclass b/meta/classes/dpkg-source.bbclass
>> index 7fd5d2ed..005eafbe 100644
>> --- a/meta/classes/dpkg-source.bbclass
>> +++ b/meta/classes/dpkg-source.bbclass
>> @@ -21,7 +21,7 @@ do_deploy_source[dirs] = "${S}"
>>  do_deploy_source() {
>>      repo_del_srcpackage "${REPO_ISAR_DIR}"/"${DISTRO}" \
>>          "${REPO_ISAR_DB_DIR}"/"${DISTRO}" "${DEBDISTRONAME}" "${BPN}"
>> -    find "${S}/../" -name '*\.dsc' -maxdepth 1 | while read package; do
>> +    find "${WORKDIR}" -maxdepth 1 -name '*\.dsc' | while read package; do
>>          repo_add_srcpackage "${REPO_ISAR_DIR}"/"${DISTRO}" \
>>              "${REPO_ISAR_DB_DIR}"/"${DISTRO}" \
>>              "${DEBDISTRONAME}" \
> 
> The longer I look at this... Why is this a loop over potentially
> multiple dsc files? We are expecting a single source package, and that
> is assumed to be called ${BPN} (that assumption for cleaning may be
> wrong with external source packages, though). So there should be also
> only a single match for *_*.dsc, no?
> 
> I'm actually trying the clean the mess we have in dpkg_runbuild where we
> are reading debian/changlog to get the dsc name. When not building the
> source packages ourselves, we would have to unpack the sources in order
> get the changelog in order to find out the dsc name that is needed to
> unpack the source - this is nonsense.
> 

The following changes on top of the this patch is apparently working 
fine, and you can simply do

IMAGE_INSTALL += "linux-headers-${KERNEL_NAME}"

again.

diff --git a/meta/recipes-kernel/linux-module/module.inc b/meta/recipes-kernel/linux-module/module.inc
index 1cca9cfb..229e6a5c 100644
--- a/meta/recipes-kernel/linux-module/module.inc
+++ b/meta/recipes-kernel/linux-module/module.inc
@@ -17,8 +17,7 @@ PN .= "-${KERNEL_NAME}"
 
 KERNEL_IMAGE_PKG ??= "linux-image-${KERNEL_NAME}"
 KERNEL_HEADERS_PKG ??= "linux-headers-${KERNEL_NAME}"
-KERNEL_KBUILD_PKG ??= "linux-kbuild-${KERNEL_NAME}"
-DEPENDS += "${KERNEL_HEADERS_PKG} ${KERNEL_KBUILD_PKG}-native"
+DEPENDS += "${KERNEL_HEADERS_PKG}-native"
 DEBIAN_BUILD_DEPENDS = "${KERNEL_HEADERS_PKG}"
 
 SIGNATURE_KEYFILE ??= ""
diff --git a/meta/recipes-kernel/linux/linux-custom.inc b/meta/recipes-kernel/linux/linux-custom.inc
index af3504dd..00b169bc 100644
--- a/meta/recipes-kernel/linux/linux-custom.inc
+++ b/meta/recipes-kernel/linux/linux-custom.inc
@@ -113,11 +113,19 @@ BUILD_PROFILES:cross-profile = "kernel"
 
 # -native: kbuild package for host
 BUILD_PROFILES:class-native = "kbuild"
-RECIPE_PROVIDES:class-native = "linux-kbuild-${KERNEL_NAME_PROVIDED}-native"
+RECIPE_PROVIDES:class-native = " \
+    linux-headers-${KERNEL_NAME_PROVIDED}-native \
+    linux-kbuild-${KERNEL_NAME_PROVIDED}-native"
+# Use pseudo target. We cannot use ${BPN} because it will be auto-extended
+# with -native by multiarch.bbclass.
+RDEPENDS:class-native = "${BPN}-pseudo-native"
 
 # -kbuildtarget: kbuild package for target, enforcing non-cross-build
 BUILD_PROFILES:class-kbuildtarget = "kbuild"
-RECIPE_PROVIDES:class-kbuildtarget = "linux-kbuild-${KERNEL_NAME_PROVIDED}"
+RECIPE_PROVIDES:class-kbuildtarget = " \
+    linux-headers-${KERNEL_NAME_PROVIDED} \
+    linux-kbuild-${KERNEL_NAME_PROVIDED}"
+RDEPENDS:class-kbuildtarget = "${BPN}"
 ISAR_CROSS_COMPILE:class-kbuildtarget = "0"
 
 # Make bitbake know we will be producing linux-image and linux-headers packages
@@ -125,15 +133,21 @@ ISAR_CROSS_COMPILE:class-kbuildtarget = "0"
 RECIPE_PROVIDES = " \
     linux-image-${KERNEL_NAME_PROVIDED} \
     linux-headers-${KERNEL_NAME_PROVIDED} \
+    linux-headers-${KERNEL_NAME_PROVIDED}-native \
     linux-libc-dev \
     linux-libc-dev-${DISTRO_ARCH}-cross \
     linux-image-${KERNEL_NAME_PROVIDED}-dbg \
     linux-kbuild-${KERNEL_NAME_PROVIDED} \
+    ${BPN}-pseudo-native \
 "
 # When cross-profile is active:
-# kbuild package is provided by -native or -kbuildtarget variant
-# Otherwise it's provided by the default variant
-RECIPE_PROVIDES:remove:cross-profile = "linux-kbuild-${KERNEL_NAME_PROVIDED}"
+# kbuild package is provided by -native or -kbuildtarget variant. Also headers
+# provisioning moves over to ensure those variants are pulled, although the
+# package itself is still built by the base recipe.
+RECIPE_PROVIDES:remove:cross-profile = " \
+    linux-headers-${KERNEL_NAME_PROVIDED} \
+    linux-headers-${KERNEL_NAME_PROVIDED}-native \
+    linux-kbuild-${KERNEL_NAME_PROVIDED}"
 
 # Append headers depends
 HEADERS_DEPENDS = ", linux-kbuild-${KERNEL_NAME_PROVIDED} | linux-kbuild-${KERNEL_NAME_PROVIDED}-${DISTRO_ARCH}-cross"


Jan
Jan Kiszka May 13, 2024, 7:01 a.m. UTC | #4
On 13.05.24 08:56, 'Jan Kiszka' via isar-users wrote:
> On 10.05.24 19:32, Jan Kiszka wrote:
>> On 05.05.24 22:32, 'Jan Kiszka' via isar-users wrote:
>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>
>>> This failed if S was not a direct subdir of WORKDIR. Align it with
>>> do_dpkg_source.
>>>
>>> Furthermore, move -maxdepth before -name to prevent warnings about a
>>> global option specified after a positional one.
>>>
>>> Fixes: 38b832ad8248 ("meta: Implement two stage build")
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> ---
>>>  meta/classes/dpkg-source.bbclass | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/meta/classes/dpkg-source.bbclass b/meta/classes/dpkg-source.bbclass
>>> index 7fd5d2ed..005eafbe 100644
>>> --- a/meta/classes/dpkg-source.bbclass
>>> +++ b/meta/classes/dpkg-source.bbclass
>>> @@ -21,7 +21,7 @@ do_deploy_source[dirs] = "${S}"
>>>  do_deploy_source() {
>>>      repo_del_srcpackage "${REPO_ISAR_DIR}"/"${DISTRO}" \
>>>          "${REPO_ISAR_DB_DIR}"/"${DISTRO}" "${DEBDISTRONAME}" "${BPN}"
>>> -    find "${S}/../" -name '*\.dsc' -maxdepth 1 | while read package; do
>>> +    find "${WORKDIR}" -maxdepth 1 -name '*\.dsc' | while read package; do
>>>          repo_add_srcpackage "${REPO_ISAR_DIR}"/"${DISTRO}" \
>>>              "${REPO_ISAR_DB_DIR}"/"${DISTRO}" \
>>>              "${DEBDISTRONAME}" \
>>
>> The longer I look at this... Why is this a loop over potentially
>> multiple dsc files? We are expecting a single source package, and that
>> is assumed to be called ${BPN} (that assumption for cleaning may be
>> wrong with external source packages, though). So there should be also
>> only a single match for *_*.dsc, no?
>>
>> I'm actually trying the clean the mess we have in dpkg_runbuild where we
>> are reading debian/changlog to get the dsc name. When not building the
>> source packages ourselves, we would have to unpack the sources in order
>> get the changelog in order to find out the dsc name that is needed to
>> unpack the source - this is nonsense.
>>
> 
> The following changes on top of the this patch is apparently working 
> fine, and you can simply do
> 
> IMAGE_INSTALL += "linux-headers-${KERNEL_NAME}"
> 
> again.
> 

[sorry, commented on the wrong thread]

Jan
Jan Kiszka May 13, 2024, 7:29 a.m. UTC | #5
On 10.05.24 19:55, 'Jan Kiszka' via isar-users wrote:
> On 10.05.24 19:32, Jan Kiszka wrote:
>> On 05.05.24 22:32, 'Jan Kiszka' via isar-users wrote:
>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>
>>> This failed if S was not a direct subdir of WORKDIR. Align it with
>>> do_dpkg_source.
>>>
>>> Furthermore, move -maxdepth before -name to prevent warnings about a
>>> global option specified after a positional one.
>>>
>>> Fixes: 38b832ad8248 ("meta: Implement two stage build")
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> ---
>>>  meta/classes/dpkg-source.bbclass | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/meta/classes/dpkg-source.bbclass b/meta/classes/dpkg-source.bbclass
>>> index 7fd5d2ed..005eafbe 100644
>>> --- a/meta/classes/dpkg-source.bbclass
>>> +++ b/meta/classes/dpkg-source.bbclass
>>> @@ -21,7 +21,7 @@ do_deploy_source[dirs] = "${S}"
>>>  do_deploy_source() {
>>>      repo_del_srcpackage "${REPO_ISAR_DIR}"/"${DISTRO}" \
>>>          "${REPO_ISAR_DB_DIR}"/"${DISTRO}" "${DEBDISTRONAME}" "${BPN}"
>>> -    find "${S}/../" -name '*\.dsc' -maxdepth 1 | while read package; do
>>> +    find "${WORKDIR}" -maxdepth 1 -name '*\.dsc' | while read package; do
>>>          repo_add_srcpackage "${REPO_ISAR_DIR}"/"${DISTRO}" \
>>>              "${REPO_ISAR_DB_DIR}"/"${DISTRO}" \
>>>              "${DEBDISTRONAME}" \
>>
>> The longer I look at this... Why is this a loop over potentially
>> multiple dsc files? We are expecting a single source package, and that
>> is assumed to be called ${BPN} (that assumption for cleaning may be
>> wrong with external source packages, though). So there should be also
>> only a single match for *_*.dsc, no?
>>
> 
> Ok, there is a case where there could be multiple dsc files: If someone
> fetches apt://foo in order to build bar.deb. Then we will not only
> deploy the sources of bar to isar-apt but also those of the dependency
> foo - ouch.
> 
> We need to figure out the source name of the target package differently.
> Not so easy...
> 

I'm inclined to assume "Source: ${BPN}" and warn during dpkg_source in
case this assumption is violated.

First, this would allow to drop all those retrieval operations later on
using debian/changelog or worse. Second, I see no other way to get the
information to a recipe that fetches the sources from isar-apt. That
recipe would otherwise have to recreate debian/changelog beforehand,
just to fetch it along with the source package.

Thoughts?

Jan

Patch

diff --git a/meta/classes/dpkg-source.bbclass b/meta/classes/dpkg-source.bbclass
index 7fd5d2ed..005eafbe 100644
--- a/meta/classes/dpkg-source.bbclass
+++ b/meta/classes/dpkg-source.bbclass
@@ -21,7 +21,7 @@  do_deploy_source[dirs] = "${S}"
 do_deploy_source() {
     repo_del_srcpackage "${REPO_ISAR_DIR}"/"${DISTRO}" \
         "${REPO_ISAR_DB_DIR}"/"${DISTRO}" "${DEBDISTRONAME}" "${BPN}"
-    find "${S}/../" -name '*\.dsc' -maxdepth 1 | while read package; do
+    find "${WORKDIR}" -maxdepth 1 -name '*\.dsc' | while read package; do
         repo_add_srcpackage "${REPO_ISAR_DIR}"/"${DISTRO}" \
             "${REPO_ISAR_DB_DIR}"/"${DISTRO}" \
             "${DEBDISTRONAME}" \