[v3,06/16] sdk: Add support for adding self-defined sdk packages

Message ID f594180ec3bcaa9f743b113c88f827e7521dc33a.1600788534.git.jan.kiszka@siemens.com
State Superseded, archived
Headers show
Series Complete backlog: SDK, compat arch, assorting fixed and cleanups | expand

Commit Message

Jan Kiszka Sept. 22, 2020, 7:28 a.m. UTC
From: Jan Kiszka <jan.kiszka@siemens.com>

We do not yet have a good algorithm for automatically adding build
dependencies to the sdk beyond the basic set, let's allow users to
append what they need by appending SDK_PREINSTALL. Analogously to other
images, also allow to install self-built packages, consequently using
SDK_INSTALL.

Based on original patch by Le Jin.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 doc/user_manual.md                           |  1 +
 meta/recipes-devtools/sdkchroot/sdkchroot.bb | 25 ++++++++++++--------
 2 files changed, 16 insertions(+), 10 deletions(-)

Comments

Baurzhan Ismagulov Sept. 29, 2020, 11:15 a.m. UTC | #1
Hello Jan,

On Tue, Sep 22, 2020 at 05:28:44PM +0200, Jan Kiszka wrote:
> We do not yet have a good algorithm for automatically adding build
> dependencies to the sdk beyond the basic set, let's allow users to
> append what they need by appending SDK_PREINSTALL. Analogously to other
> images, also allow to install self-built packages, consequently using
> SDK_INSTALL.
> 
> Based on original patch by Le Jin.

Two issues:

1. I've tried the following:

   . ./isar-init-build-env build
   bitbake -c do_populate_sdk mc:qemuarm-buster:isar-image-base

   When I look into tmp/deploy/images/qemuarm/sdk-debian-buster-armhf.tar.xz,
   etc/apt/sources.list.d/isar-apt.list is not present. Am I missing anything?

2. The series still removes isar-apt description in p11.

With kind regards,
Baurzhan.
Jan Kiszka Sept. 30, 2020, 11:19 a.m. UTC | #2
On 29.09.20 21:15, Baurzhan Ismagulov wrote:
> Hello Jan,
>
> On Tue, Sep 22, 2020 at 05:28:44PM +0200, Jan Kiszka wrote:
>> We do not yet have a good algorithm for automatically adding build
>> dependencies to the sdk beyond the basic set, let's allow users to
>> append what they need by appending SDK_PREINSTALL. Analogously to other
>> images, also allow to install self-built packages, consequently using
>> SDK_INSTALL.
>>
>> Based on original patch by Le Jin.
>
> Two issues:
>
> 1. I've tried the following:
>
>    . ./isar-init-build-env build
>    bitbake -c do_populate_sdk mc:qemuarm-buster:isar-image-base
>
>    When I look into tmp/deploy/images/qemuarm/sdk-debian-buster-armhf.tar.xz,
>    etc/apt/sources.list.d/isar-apt.list is not present. Am I missing anything?

Was that with SDK_INCLUDE_ISAR_APT = "1"? It's opt-in now, see patch 9.

>
> 2. The series still removes isar-apt description in p11.

It's opt-in now, the common case for most users remains setting
SDK_INSTALL. We are only leaving the option to ship all the packages,
including all the useless ones, for special purposes.

Jan
Baurzhan Ismagulov Oct. 6, 2020, midnight UTC | #3
On Wed, Sep 30, 2020 at 09:19:15PM +0200, Jan Kiszka wrote:
> > 1. I've tried the following:
> >
> >    . ./isar-init-build-env build
> >    bitbake -c do_populate_sdk mc:qemuarm-buster:isar-image-base
> >
> >    When I look into tmp/deploy/images/qemuarm/sdk-debian-buster-armhf.tar.xz,
> >    etc/apt/sources.list.d/isar-apt.list is not present. Am I missing anything?
> 
> Was that with SDK_INCLUDE_ISAR_APT = "1"? It's opt-in now, see patch 9.

Yes, I forgot to mention that, sorry for the confusion. The commands I used,
hopefully correct now:

. ./isar-init-build-env build
echo 'SDK_INCLUDE_ISAR_APT = "1"' >>conf/local.conf
echo 'ISAR_CROSS_COMPILE = "1"' >>conf/local.conf
time bitbake -c do_populate_sdk mc:qemuarm-buster:isar-image-base
cd tmp/deploy/images/qemuarm
sudo tar Jxf sdk-debian-buster-armhf.tar.xz
ls sdk-debian-buster-armhf/etc/apt/sources.list.d/isar-apt.list
ls: cannot access 'sdk-debian-buster-armhf/etc/apt/sources.list.d/isar-apt.list': No such file or directory


> > 2. The series still removes isar-apt description in p11.
> 
> It's opt-in now, the common case for most users remains setting
> SDK_INSTALL. We are only leaving the option to ship all the packages,
> including all the useless ones, for special purposes.

I'm referring to the following hunk:

- - Check that cross toolchains are installed
-
-:~# dpkg -l | grep crossbuild-essential-armhf
-ii  crossbuild-essential-armhf           12.3                   all          Informational list of cross-build-essential packages
-
- - Install needed prebuilt target packages.
-
-:~# apt-get update
-:~# apt-get install libhello-dev:armhf
-
- - Check the contents of the installed target package
-
-:~# dpkg -L libhello-dev
-/.
-/usr
-/usr/include
-/usr/include/hello.h
-/usr/lib
-/usr/lib/arm-linux-gnueabihf
-/usr/lib/arm-linux-gnueabihf/libhello.a
-/usr/lib/arm-linux-gnueabihf/libhello.la
-/usr/share
-/usr/share/doc
-/usr/share/doc/libhello-dev
-/usr/share/doc/libhello-dev/changelog.gz
-/usr/share/doc/libhello-dev/copyright
-~#

This explains to the user how Debian multiarch works with option 2. This
example works inside or outside of the chroot. We should not be removing
documentation for a supported use case, thus NAK here.


3. In 08/16: Suggest "conpilation" -> "compilation".


With kind regards,
Baurzhan.
Jan Kiszka Oct. 6, 2020, 10:30 p.m. UTC | #4
On 06.10.20 11:00, Baurzhan Ismagulov wrote:
> On Wed, Sep 30, 2020 at 09:19:15PM +0200, Jan Kiszka wrote:
>>> 1. I've tried the following:
>>>
>>>    . ./isar-init-build-env build
>>>    bitbake -c do_populate_sdk mc:qemuarm-buster:isar-image-base
>>>
>>>    When I look into tmp/deploy/images/qemuarm/sdk-debian-buster-armhf.tar.xz,
>>>    etc/apt/sources.list.d/isar-apt.list is not present. Am I missing anything?
>>
>> Was that with SDK_INCLUDE_ISAR_APT = "1"? It's opt-in now, see patch 9.
> 
> Yes, I forgot to mention that, sorry for the confusion. The commands I used,
> hopefully correct now:
> 
> . ./isar-init-build-env build
> echo 'SDK_INCLUDE_ISAR_APT = "1"' >>conf/local.conf
> echo 'ISAR_CROSS_COMPILE = "1"' >>conf/local.conf
> time bitbake -c do_populate_sdk mc:qemuarm-buster:isar-image-base
> cd tmp/deploy/images/qemuarm
> sudo tar Jxf sdk-debian-buster-armhf.tar.xz
> ls sdk-debian-buster-armhf/etc/apt/sources.list.d/isar-apt.list
> ls: cannot access 'sdk-debian-buster-armhf/etc/apt/sources.list.d/isar-apt.list': No such file or directory
> 

OK, need to check again. Cannot explain yet why that should happen,
given the change in patch 9. Maybe I have a regression further down the
series...

> 
>>> 2. The series still removes isar-apt description in p11.
>>
>> It's opt-in now, the common case for most users remains setting
>> SDK_INSTALL. We are only leaving the option to ship all the packages,
>> including all the useless ones, for special purposes.
> 
> I'm referring to the following hunk:
> 
> - - Check that cross toolchains are installed
> -
> -:~# dpkg -l | grep crossbuild-essential-armhf
> -ii  crossbuild-essential-armhf           12.3                   all          Informational list of cross-build-essential packages
> -
> - - Install needed prebuilt target packages.
> -
> -:~# apt-get update
> -:~# apt-get install libhello-dev:armhf
> -
> - - Check the contents of the installed target package
> -
> -:~# dpkg -L libhello-dev
> -/.
> -/usr
> -/usr/include
> -/usr/include/hello.h
> -/usr/lib
> -/usr/lib/arm-linux-gnueabihf
> -/usr/lib/arm-linux-gnueabihf/libhello.a
> -/usr/lib/arm-linux-gnueabihf/libhello.la
> -/usr/share
> -/usr/share/doc
> -/usr/share/doc/libhello-dev
> -/usr/share/doc/libhello-dev/changelog.gz
> -/usr/share/doc/libhello-dev/copyright
> -~#
> 
> This explains to the user how Debian multiarch works with option 2. This
> example works inside or outside of the chroot. We should not be removing
> documentation for a supported use case, thus NAK here.
> 

This hunk looks pointless to me. Why should the user try out all that
stuff? It's the duty of the SDK creator to do that upfront and ship a
properly working package.

Jan

> 
> 3. In 08/16: Suggest "conpilation" -> "compilation".
> 

Thanks, will fix.

But please comment inline, not out of context.

Jan
Baurzhan Ismagulov Oct. 6, 2020, 11:06 p.m. UTC | #5
On Wed, Oct 07, 2020 at 09:30:06AM +0200, Jan Kiszka wrote:
> OK, need to check again. Cannot explain yet why that should happen,
> given the change in patch 9. Maybe I have a regression further down the
> series...

Thanks, please let us know. I also stared at the line for some time and
couldn't see what is wrong.


> > This explains to the user how Debian multiarch works with option 2. This
> > example works inside or outside of the chroot. We should not be removing
> > documentation for a supported use case, thus NAK here.
> > 
> 
> This hunk looks pointless to me. Why should the user try out all that
> stuff? It's the duty of the SDK creator to do that upfront and ship a
> properly working package.

Trying in the context of SDK -- that's a matter of taste, I like when some
background is presented rather than "do 1, 2, 3", sometimes without
understanding what I'm really doing. The hunk explains how Debian officially
handles cross-building with multiarch, so I'd be reluctant to remove that
completely. Maybe we could make it a separate section on Debian basics or
details how SDK option 2 is implemented.


> Thanks, will fix.
> 
> But please comment inline, not out of context.

I wanted to avoid scattering the issue over many mails -- but no problem, I can
do it.


With kind regards,
Baurzhan.
Jan Kiszka Oct. 7, 2020, 4:36 a.m. UTC | #6
On 07.10.20 10:06, Baurzhan Ismagulov wrote:
> On Wed, Oct 07, 2020 at 09:30:06AM +0200, Jan Kiszka wrote:
>> OK, need to check again. Cannot explain yet why that should happen,
>> given the change in patch 9. Maybe I have a regression further down the
>> series...
> 
> Thanks, please let us know. I also stared at the line for some time and
> couldn't see what is wrong.
> 

Fixed: test with "==", rather than "=". Cleaned up more around that.
Will send v4.

> 
>>> This explains to the user how Debian multiarch works with option 2. This
>>> example works inside or outside of the chroot. We should not be removing
>>> documentation for a supported use case, thus NAK here.
>>>
>>
>> This hunk looks pointless to me. Why should the user try out all that
>> stuff? It's the duty of the SDK creator to do that upfront and ship a
>> properly working package.
> 
> Trying in the context of SDK -- that's a matter of taste, I like when some
> background is presented rather than "do 1, 2, 3", sometimes without
> understanding what I'm really doing. The hunk explains how Debian officially
> handles cross-building with multiarch, so I'd be reluctant to remove that
> completely. Maybe we could make it a separate section on Debian basics or
> details how SDK option 2 is implemented.

I'm all for adding relevant information, but this one does not qualify.
Checking for the toolchain being install was always completely pointless
in the context of an SDK that always has this pre-installed according to
our recipes. And since isar-apt is now a special case and SDK_INSTALL
will ensure that all required packages are already installed, the second
hint is also irrelevant for normal users. It is only for the case of
SDK_INCLUDE_ISAR_APT = 1 *and* SDK_INSTALL *not* listing self-built
devel packages needed for building applications.

If you have other information that may help users with using the SDK in
chroot mode, feel free to add that on top.

Jan
Baurzhan Ismagulov Oct. 15, 2020, 12:51 p.m. UTC | #7
On Wed, Oct 07, 2020 at 03:36:47PM +0200, Jan Kiszka wrote:
> Fixed: test with "==", rather than "=". Cleaned up more around that.

Thanks, isar-apt.list is now available. However, when I enter the chroot, an
attempt to run arm-linux-gnueabihf-gcc results in:

-bash: arm-linux-gnueabihf-gcc: command not found


> I'm all for adding relevant information, but this one does not qualify.
> Checking for the toolchain being install was always completely pointless
> in the context of an SDK that always has this pre-installed according to
> our recipes.

Yes, it might be confusing in the context of SDK. The section is meant as a
story and not a literal instruction. It tells the user that he can sudo apt-get
install crossbuild-essential-armhf libhello-dev:armhf and build his application
on the host with stock Debian, without any SDK. It describes the official
Debian build tooling and is certainly not limited to the chroot mode, while the
SDK is a convenience offer so that developers famliar with Yocto can start
right away before they learn Debian tools. Checking for the toolchain shows how
stuff works rather than verifying that the recipes have done their job.


> And since isar-apt is now a special case and SDK_INSTALL
> will ensure that all required packages are already installed, the second
> hint is also irrelevant for normal users. It is only for the case of
> SDK_INCLUDE_ISAR_APT = 1 *and* SDK_INSTALL *not* listing self-built
> devel packages needed for building applications.

Yes, so, it is a supported use case and must remain documented.


> If you have other information that may help users with using the SDK in
> chroot mode, feel free to add that on top.

I don't have new material to add. The existing wording is good enough for me.
After the compiler is fixed, I'd be ready to apply the series while keeping the
discussed section till we have a suitable replacement.


With kind regards,
Baurzhan.
Jan Kiszka Oct. 15, 2020, 10:15 p.m. UTC | #8
On 15.10.20 23:51, Baurzhan Ismagulov wrote:
> On Wed, Oct 07, 2020 at 03:36:47PM +0200, Jan Kiszka wrote:
>> Fixed: test with "==", rather than "=". Cleaned up more around that.
> 
> Thanks, isar-apt.list is now available. However, when I enter the chroot, an
> attempt to run arm-linux-gnueabihf-gcc results in:
> 
> -bash: arm-linux-gnueabihf-gcc: command not found
> 

Checking... Nope, works fine here:

- echo 'SDK_INCLUDE_ISAR_APT = "1"' >> conf/local.conf
- bitbake -c populate_sdk mc:qemuarm-buster:isar-image-base
- sudo tar xJf
/work/build/tmp/deploy/images/qemuarm/sdk-debian-buster-armhf.tar.xz
- sudo sdk-debian-buster-armhf/mount_chroot.sh sdk-debian-buster-armhf/
- sudo chroot sdk-debian-buster-armhf/

root@566b0c868313:/# arm-linux-gnueabihf-gcc
arm-linux-gnueabihf-gcc-8.bin: fatal error: no input files
compilation terminated.

Please provide more details what you did.

> 
>> I'm all for adding relevant information, but this one does not qualify.
>> Checking for the toolchain being install was always completely pointless
>> in the context of an SDK that always has this pre-installed according to
>> our recipes.
> 
> Yes, it might be confusing in the context of SDK. The section is meant as a
> story and not a literal instruction. It tells the user that he can sudo apt-get
> install crossbuild-essential-armhf libhello-dev:armhf and build his application
> on the host with stock Debian, without any SDK. It describes the official

This section is part of the SDK, not the user manual. So the context is
not as broad as you describe it here.

> Debian build tooling and is certainly not limited to the chroot mode, while the
> SDK is a convenience offer so that developers famliar with Yocto can start
> right away before they learn Debian tools. Checking for the toolchain shows how
> stuff works rather than verifying that the recipes have done their job.
> 
> 
>> And since isar-apt is now a special case and SDK_INSTALL
>> will ensure that all required packages are already installed, the second
>> hint is also irrelevant for normal users. It is only for the case of
>> SDK_INCLUDE_ISAR_APT = 1 *and* SDK_INSTALL *not* listing self-built
>> devel packages needed for building applications.
> 
> Yes, so, it is a supported use case and must remain documented.

Then we need README generation - because all this only makes sense in
that non-default case. I don't see much value, though.

> 
> 
>> If you have other information that may help users with using the SDK in
>> chroot mode, feel free to add that on top.
> 
> I don't have new material to add. The existing wording is good enough for me.
> After the compiler is fixed, I'd be ready to apply the series while keeping the
> discussed section till we have a suitable replacement.

No, that section will not stay for the common case. I will add the
relevant parts in case SDK_INCLUDE_ISAR_APT = 1 and still drop the
irrelevant ones (toolchain check).

Jan
Baurzhan Ismagulov Nov. 1, 2020, 5:17 a.m. UTC | #9
On Fri, Oct 16, 2020 at 09:15:12AM +0200, Jan Kiszka wrote:
> Checking... Nope, works fine here:
> 
> - echo 'SDK_INCLUDE_ISAR_APT = "1"' >> conf/local.conf
> - bitbake -c populate_sdk mc:qemuarm-buster:isar-image-base
> - sudo tar xJf
> /work/build/tmp/deploy/images/qemuarm/sdk-debian-buster-armhf.tar.xz
> - sudo sdk-debian-buster-armhf/mount_chroot.sh sdk-debian-buster-armhf/
> - sudo chroot sdk-debian-buster-armhf/
> 
> root@566b0c868313:/# arm-linux-gnueabihf-gcc
> arm-linux-gnueabihf-gcc-8.bin: fatal error: no input files
> compilation terminated.

Thanks for the details! Turns out I made a mistake while applying the series.


> No, that section will not stay for the common case. I will add the
> relevant parts in case SDK_INCLUDE_ISAR_APT = 1 and still drop the
> irrelevant ones (toolchain check).

As discussed offline, I'd like to have a consensus here. To summarize, your
point is that the description is not necessary if README.sdk is used as a
literal howto, and that the deeper message is only clear for expert users, who
know that anyway. Given these questions, I agree with the answers. My question
is a different one: Multiarch is Debian's unique feature for embedded
development, and we should be covering that somewhere in the docs. As a
solution, I'd like to enhance the existing docs instead of removing them. I've
checked the user manual, the topics are covered there. I've applied v4. As
agreed, the enhancements can still be implemented, I can provide them on top.


With kind regards,
Baurzhan.

Patch

diff --git a/doc/user_manual.md b/doc/user_manual.md
index d13a74e9..fb6574bb 100644
--- a/doc/user_manual.md
+++ b/doc/user_manual.md
@@ -744,6 +744,7 @@  target binary artifacts. Developer chroots to sdk rootfs and develops applicatio
 
 User manually triggers creation of SDK root filesystem for his target platform by launching the task `do_populate_sdk` for target image, f.e.
 `bitbake -c do_populate_sdk mc:${MACHINE}-${DISTRO}:isar-image-base`.
+Packages that should be additionally installed into the SDK can be appended to `SDK_PREINSTALL` (external repositories) and `SDK_INSTALL` (self-built).
 
 The resulting SDK rootfs is archived into `tmp/deploy/images/${MACHINE}/sdk-${DISTRO}-${DISTRO_ARCH}.tar.xz`.
 It is additionally available for direct use under `tmp/deploy/images/${MACHINE}/sdk-${DISTRO}-${DISTRO_ARCH}/`.
diff --git a/meta/recipes-devtools/sdkchroot/sdkchroot.bb b/meta/recipes-devtools/sdkchroot/sdkchroot.bb
index 467e6824..ab0a66dc 100644
--- a/meta/recipes-devtools/sdkchroot/sdkchroot.bb
+++ b/meta/recipes-devtools/sdkchroot/sdkchroot.bb
@@ -13,6 +13,10 @@  SRC_URI = " \
     file://README.sdk"
 PV = "0.1"
 
+SDK_INSTALL ?= ""
+
+DEPENDS += "${SDK_INSTALL}"
+
 TOOLCHAIN = "crossbuild-essential-${DISTRO_ARCH}"
 TOOLCHAIN_${HOST_ARCH} = "build-essential"
 TOOLCHAIN_i386 = "build-essential"
@@ -21,7 +25,7 @@  inherit rootfs
 ROOTFS_ARCH = "${HOST_ARCH}"
 ROOTFS_DISTRO = "${HOST_DISTRO}"
 ROOTFSDIR = "${S}"
-ROOTFS_PACKAGES = "${SDKCHROOT_PREINSTALL} ${TOOLCHAIN}"
+ROOTFS_PACKAGES = "${SDK_PREINSTALL} ${SDK_INSTALL} ${TOOLCHAIN}"
 ROOTFS_FEATURES += "clean-package-cache generate-manifest"
 ROOTFS_MANIFEST_DEPLOY_DIR = "${DEPLOY_DIR_SDKCHROOT}"
 
@@ -31,15 +35,16 @@  python() {
             d.getVar("ROOTFS_ARCH")))
 }
 
-SDKCHROOT_PREINSTALL := "debhelper \
-                           autotools-dev \
-                           dpkg \
-                           locales \
-                           docbook-to-man \
-                           apt \
-                           automake \
-                           devscripts \
-                           equivs"
+SDK_PREINSTALL += " \
+    debhelper \
+    autotools-dev \
+    dpkg \
+    locales \
+    docbook-to-man \
+    apt \
+    automake \
+    devscripts \
+    equivs"
 
 S = "${WORKDIR}/rootfs"