diff mbox series

[1/2] package-index: Convert package-index into a bbclass.

Message ID 20230524215526.323283-1-charlie.johnston@ni.com
State New
Headers show
Series [1/2] package-index: Convert package-index into a bbclass. | expand

Commit Message

Charlie Johnston May 24, 2023, 9:55 p.m. UTC
Currently, package-index is a bit strange because it's a
bitbake recipe that implements a task. That task is
already used elsewhere and could be leveraged further
for BUILD_IMAGES_FROM_FEEDS.

This change moves the logic for do_package_index into its
own bbclass and has package-index.bb inherit from there.

Signed-off-by: Charlie Johnston <charlie.johnston@ni.com>
---
 meta/classes/package_index.bbclass      | 21 +++++++++++++++++++++
 meta/recipes-core/meta/package-index.bb | 10 +---------
 2 files changed, 22 insertions(+), 9 deletions(-)
 create mode 100644 meta/classes/package_index.bbclass

Comments

Richard Purdie May 24, 2023, 11:26 p.m. UTC | #1
On Wed, 2023-05-24 at 16:55 -0500, Charlie Johnston wrote:
> Currently, package-index is a bit strange because it's a
> bitbake recipe that implements a task.

That isn't strange at all, it is perfectly fine for a recipe to add a
task.

> That task is
> already used elsewhere and could be leveraged further
> for BUILD_IMAGES_FROM_FEEDS.
> 
> This change moves the logic for do_package_index into its
> own bbclass and has package-index.bb inherit from there.

The description here does not match what this patch actually does. The
patch changes the behaviour of the code considerably and this isn't
even mentioned.

> 
> Signed-off-by: Charlie Johnston <charlie.johnston@ni.com>
> ---
>  meta/classes/package_index.bbclass      | 21 +++++++++++++++++++++
>  meta/recipes-core/meta/package-index.bb | 10 +---------
>  2 files changed, 22 insertions(+), 9 deletions(-)
>  create mode 100644 meta/classes/package_index.bbclass
> 
> diff --git a/meta/classes/package_index.bbclass b/meta/classes/package_index.bbclass
> new file mode 100644
> index 0000000000..fdd10f6dd0
> --- /dev/null
> +++ b/meta/classes/package_index.bbclass
> @@ -0,0 +1,21 @@
> +#
> +#   Creates package indices for the IMAGE_PKGTYPE
> +#
> +
> +do_package_index[nostamp] = "1"
> +do_package_index[depends] += "${PACKAGEINDEXDEPS}"
> +do_package_index[recrdeptask] += 'do_package_write_deb'
> +do_package_index[recrdeptask] += 'do_package_write_ipk'
> +do_package_index[recrdeptask] += 'do_package_write_rpm'

Adding recrdeptask isn't mentioned in the commit message. "package-
index" deliberately just indexes what is in the packages directory with
no dependencies. This is therefore a significant change in behaviour.

> +
> +python do_package_index() {
> +    from oe.rootfs import generate_index_files
> +    generate_index_files(d)
> +}
> +
> +# Package indexes are required for the dummy SDK architectures
> +# to support scenarios where SDK images are built from feeds.
> +PACKAGE_ARCHS:append:task-package-index = " sdk-provides-dummy-target"
> +SDK_PACKAGE_ARCHS:append:task-package-index = " sdk-provides-dummy-${SDKPKGSUFFIX}"

This is also a change in behaviour, again unmentioned.

Cheers,

Richard

> +
> +addtask do_package_index before do_build
> diff --git a/meta/recipes-core/meta/package-index.bb b/meta/recipes-core/meta/package-index.bb
> index 98c5bcb372..3fc18865b7 100644
> --- a/meta/recipes-core/meta/package-index.bb
> +++ b/meta/recipes-core/meta/package-index.bb
> @@ -4,7 +4,7 @@ LICENSE = "MIT"
>  INHIBIT_DEFAULT_DEPS = "1"
>  PACKAGES = ""
>  
> -inherit nopackages
> +inherit nopackages package_index
>  
>  deltask do_fetch
>  deltask do_unpack
> @@ -15,12 +15,4 @@ deltask do_install
>  deltask do_populate_lic
>  deltask do_populate_sysroot
>  
> -do_package_index[nostamp] = "1"
> -do_package_index[depends] += "${PACKAGEINDEXDEPS}"
> -
> -python do_package_index() {
> -    from oe.rootfs import generate_index_files
> -    generate_index_files(d)
> -}
> -addtask do_package_index before do_build
>  EXCLUDE_FROM_WORLD = "1"
Charlie Johnston May 25, 2023, 3:59 p.m. UTC | #2
Thanks for the feedback. Are there concerns on this specific patch you have other than the commit message?

Thanks,
Charlie

On Wed, May 24, 2023 at 06:26 PM, Richard Purdie wrote:

> 
> On Wed, 2023-05-24 at 16:55 -0500, Charlie Johnston wrote:
> 
>> Currently, package-index is a bit strange because it's a
>> bitbake recipe that implements a task.
> 
> That isn't strange at all, it is perfectly fine for a recipe to add a
> task.
> 
> 
>> That task is
>> already used elsewhere and could be leveraged further
>> for BUILD_IMAGES_FROM_FEEDS.
>> 
>> This change moves the logic for do_package_index into its
>> own bbclass and has package-index.bb inherit from there.
> 
> The description here does not match what this patch actually does. The
> patch changes the behaviour of the code considerably and this isn't
> even mentioned.
> 
> 
>> 
>> Signed-off-by: Charlie Johnston <charlie.johnston@ni.com>
>> ---
>> meta/classes/package_index.bbclass | 21 +++++++++++++++++++++
>> meta/recipes-core/meta/package-index.bb | 10 +---------
>> 2 files changed, 22 insertions(+), 9 deletions(-)
>> create mode 100644 meta/classes/package_index.bbclass
>> 
>> diff --git a/meta/classes/package_index.bbclass
>> b/meta/classes/package_index.bbclass
>> new file mode 100644
>> index 0000000000..fdd10f6dd0
>> --- /dev/null
>> +++ b/meta/classes/package_index.bbclass
>> @@ -0,0 +1,21 @@
>> +#
>> +# Creates package indices for the IMAGE_PKGTYPE
>> +#
>> +
>> +do_package_index[nostamp] = "1"
>> +do_package_index[depends] += "${PACKAGEINDEXDEPS}"
>> +do_package_index[recrdeptask] += 'do_package_write_deb'
>> +do_package_index[recrdeptask] += 'do_package_write_ipk'
>> +do_package_index[recrdeptask] += 'do_package_write_rpm'
> 
> Adding recrdeptask isn't mentioned in the commit message. "package-
> index" deliberately just indexes what is in the packages directory with
> no dependencies. This is therefore a significant change in behaviour.
> 
> 
>> +
>> +python do_package_index() {
>> + from oe.rootfs import generate_index_files
>> + generate_index_files(d)
>> +}
>> +
>> +# Package indexes are required for the dummy SDK architectures
>> +# to support scenarios where SDK images are built from feeds.
>> +PACKAGE_ARCHS:append:task-package-index = " sdk-provides-dummy-target"
>> +SDK_PACKAGE_ARCHS:append:task-package-index = "
>> sdk-provides-dummy-${SDKPKGSUFFIX}"
> 
> This is also a change in behaviour, again unmentioned.
> 
> Cheers,
> 
> Richard
> 
> 
>> +
>> +addtask do_package_index before do_build
>> diff --git a/meta/recipes-core/meta/package-index.bb
>> b/meta/recipes-core/meta/package-index.bb
>> index 98c5bcb372..3fc18865b7 100644
>> --- a/meta/recipes-core/meta/package-index.bb
>> +++ b/meta/recipes-core/meta/package-index.bb
>> @@ -4,7 +4,7 @@ LICENSE = "MIT"
>> INHIBIT_DEFAULT_DEPS = "1"
>> PACKAGES = ""
>> 
>> -inherit nopackages
>> +inherit nopackages package_index
>> 
>> deltask do_fetch
>> deltask do_unpack
>> @@ -15,12 +15,4 @@ deltask do_install
>> deltask do_populate_lic
>> deltask do_populate_sysroot
>> 
>> -do_package_index[nostamp] = "1"
>> -do_package_index[depends] += "${PACKAGEINDEXDEPS}"
>> -
>> -python do_package_index() {
>> - from oe.rootfs import generate_index_files
>> - generate_index_files(d)
>> -}
>> -addtask do_package_index before do_build
>> EXCLUDE_FROM_WORLD = "1"
> 
>
diff mbox series

Patch

diff --git a/meta/classes/package_index.bbclass b/meta/classes/package_index.bbclass
new file mode 100644
index 0000000000..fdd10f6dd0
--- /dev/null
+++ b/meta/classes/package_index.bbclass
@@ -0,0 +1,21 @@ 
+#
+#   Creates package indices for the IMAGE_PKGTYPE
+#
+
+do_package_index[nostamp] = "1"
+do_package_index[depends] += "${PACKAGEINDEXDEPS}"
+do_package_index[recrdeptask] += 'do_package_write_deb'
+do_package_index[recrdeptask] += 'do_package_write_ipk'
+do_package_index[recrdeptask] += 'do_package_write_rpm'
+
+python do_package_index() {
+    from oe.rootfs import generate_index_files
+    generate_index_files(d)
+}
+
+# Package indexes are required for the dummy SDK architectures
+# to support scenarios where SDK images are built from feeds.
+PACKAGE_ARCHS:append:task-package-index = " sdk-provides-dummy-target"
+SDK_PACKAGE_ARCHS:append:task-package-index = " sdk-provides-dummy-${SDKPKGSUFFIX}"
+
+addtask do_package_index before do_build
diff --git a/meta/recipes-core/meta/package-index.bb b/meta/recipes-core/meta/package-index.bb
index 98c5bcb372..3fc18865b7 100644
--- a/meta/recipes-core/meta/package-index.bb
+++ b/meta/recipes-core/meta/package-index.bb
@@ -4,7 +4,7 @@  LICENSE = "MIT"
 INHIBIT_DEFAULT_DEPS = "1"
 PACKAGES = ""
 
-inherit nopackages
+inherit nopackages package_index
 
 deltask do_fetch
 deltask do_unpack
@@ -15,12 +15,4 @@  deltask do_install
 deltask do_populate_lic
 deltask do_populate_sysroot
 
-do_package_index[nostamp] = "1"
-do_package_index[depends] += "${PACKAGEINDEXDEPS}"
-
-python do_package_index() {
-    from oe.rootfs import generate_index_files
-    generate_index_files(d)
-}
-addtask do_package_index before do_build
 EXCLUDE_FROM_WORLD = "1"