fix: remove unneeded unconditional recipes

Message ID 20210413152607.201401-1-silvano.cirujano-cuesta@siemens.com
State Superseded, archived
Headers show
Series fix: remove unneeded unconditional recipes | expand

Commit Message

Silvano Cirujano Cuesta April 13, 2021, 7:26 a.m. UTC
Commit 9c3499adf91 introduced the unconditional inclusion of the recipes
isar-exclude-docs and isar-disable-apt-cache, but they are only needed
for container images.

Signed-off-by: Silvano Cirujano Cuesta <silvano.cirujano-cuesta@siemens.com>
---
 meta/classes/container-img.bbclass             | 2 ++
 meta/classes/image-container-extension.bbclass | 1 -
 2 files changed, 2 insertions(+), 1 deletion(-)

Comments

Jan Kiszka April 13, 2021, 8:01 a.m. UTC | #1
On 13.04.21 17:26, [ext] Silvano Cirujano Cuesta wrote:
> Commit 9c3499adf91 introduced the unconditional inclusion of the recipes
> isar-exclude-docs and isar-disable-apt-cache, but they are only needed
> for container images.
> 

In fact, are those packages truly needed or just desirable, reasonable
defaults?

In any case, it might be worth documenting those defaults as well. They
can be overwritten (IMAGE_INSTALL_remove), but the users should be made
aware of them.

Jan

> Signed-off-by: Silvano Cirujano Cuesta <silvano.cirujano-cuesta@siemens.com>
> ---
>  meta/classes/container-img.bbclass             | 2 ++
>  meta/classes/image-container-extension.bbclass | 1 -
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/meta/classes/container-img.bbclass b/meta/classes/container-img.bbclass
> index 79ef3e8..887d49e 100644
> --- a/meta/classes/container-img.bbclass
> +++ b/meta/classes/container-img.bbclass
> @@ -6,6 +6,8 @@
>  # This class provides the task 'containerize_rootfs'
>  # to create container images containing the target rootfs.
>  
> +IMAGE_INSTALL += "isar-exclude-docs isar-disable-apt-cache"
> +
>  do_container_image[stamp-extra-info] = "${DISTRO}-${MACHINE}"
>  do_container_image[vardeps] += "CONTAINER_FORMATS"
>  do_container_image(){
> diff --git a/meta/classes/image-container-extension.bbclass b/meta/classes/image-container-extension.bbclass
> index f693627..e26604a 100644
> --- a/meta/classes/image-container-extension.bbclass
> +++ b/meta/classes/image-container-extension.bbclass
> @@ -6,7 +6,6 @@
>  # This class extends the image.bbclass for containerizing the root filesystem.
>  
>  CONTAINER_FORMATS ?= "docker-archive"
> -IMAGE_INSTALL += "isar-exclude-docs isar-disable-apt-cache"
>  
>  containerize_rootfs() {
>      local cmd="/bin/dash"
>
Silvano Cirujano Cuesta April 13, 2021, 11:45 p.m. UTC | #2
On 13/04/2021 18:01, Jan Kiszka wrote:
> On 13.04.21 17:26, [ext] Silvano Cirujano Cuesta wrote:
>> Commit 9c3499adf91 introduced the unconditional inclusion of the recipes
>> isar-exclude-docs and isar-disable-apt-cache, but they are only needed
>> for container images.
>>
> 
> In fact, are those packages truly needed or just desirable, reasonable
> defaults?

I was yesterday about to write "reasonable defaults"...
But after having faced some difficulties to find a robust implementation to keep it, I decided to reconsider it.

After having slept over it, I think IT'S NOT EVEN A REASONABLE DEFAULT 8-o

It's a storage optimization that is only being used in "slim" images.
Having used debian "slim" images as a size reference brought me to introduce it.

So I'll send a new patch completely removing it.
If we want to later on offer that optimization as an optional feature, we can do so.

  Silvano

> 
> In any case, it might be worth documenting those defaults as well. They
> can be overwritten (IMAGE_INSTALL_remove), but the users should be made
> aware of them.
> 
> Jan
> 
>> Signed-off-by: Silvano Cirujano Cuesta <silvano.cirujano-cuesta@siemens.com>
>> ---
>>  meta/classes/container-img.bbclass             | 2 ++
>>  meta/classes/image-container-extension.bbclass | 1 -
>>  2 files changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/meta/classes/container-img.bbclass b/meta/classes/container-img.bbclass
>> index 79ef3e8..887d49e 100644
>> --- a/meta/classes/container-img.bbclass
>> +++ b/meta/classes/container-img.bbclass
>> @@ -6,6 +6,8 @@
>>  # This class provides the task 'containerize_rootfs'
>>  # to create container images containing the target rootfs.
>>  
>> +IMAGE_INSTALL += "isar-exclude-docs isar-disable-apt-cache"
>> +
>>  do_container_image[stamp-extra-info] = "${DISTRO}-${MACHINE}"
>>  do_container_image[vardeps] += "CONTAINER_FORMATS"
>>  do_container_image(){
>> diff --git a/meta/classes/image-container-extension.bbclass b/meta/classes/image-container-extension.bbclass
>> index f693627..e26604a 100644
>> --- a/meta/classes/image-container-extension.bbclass
>> +++ b/meta/classes/image-container-extension.bbclass
>> @@ -6,7 +6,6 @@
>>  # This class extends the image.bbclass for containerizing the root filesystem.
>>  
>>  CONTAINER_FORMATS ?= "docker-archive"
>> -IMAGE_INSTALL += "isar-exclude-docs isar-disable-apt-cache"
>>  
>>  containerize_rootfs() {
>>      local cmd="/bin/dash"
>>
>

Patch

diff --git a/meta/classes/container-img.bbclass b/meta/classes/container-img.bbclass
index 79ef3e8..887d49e 100644
--- a/meta/classes/container-img.bbclass
+++ b/meta/classes/container-img.bbclass
@@ -6,6 +6,8 @@ 
 # This class provides the task 'containerize_rootfs'
 # to create container images containing the target rootfs.
 
+IMAGE_INSTALL += "isar-exclude-docs isar-disable-apt-cache"
+
 do_container_image[stamp-extra-info] = "${DISTRO}-${MACHINE}"
 do_container_image[vardeps] += "CONTAINER_FORMATS"
 do_container_image(){
diff --git a/meta/classes/image-container-extension.bbclass b/meta/classes/image-container-extension.bbclass
index f693627..e26604a 100644
--- a/meta/classes/image-container-extension.bbclass
+++ b/meta/classes/image-container-extension.bbclass
@@ -6,7 +6,6 @@ 
 # This class extends the image.bbclass for containerizing the root filesystem.
 
 CONTAINER_FORMATS ?= "docker-archive"
-IMAGE_INSTALL += "isar-exclude-docs isar-disable-apt-cache"
 
 containerize_rootfs() {
     local cmd="/bin/dash"