[meta-oe][PATCH v4] iproute2: split ip to individual package

[meta-oe][PATCH v4] iproute2: split ip to individual package

[Sinan Kaya]

Move the ip tool into its own package. Useful for size constrained
systems that only want the ip tool.

Signed-off-by: Sinan Kaya <okaya@kernel.org>
---
 meta/recipes-connectivity/iproute2/iproute2.inc | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/meta/recipes-connectivity/iproute2/iproute2.inc b/meta/recipes-connectivity/iproute2/iproute2.inc
index 403d264308c..44f336bd580 100644
--- a/meta/recipes-connectivity/iproute2/iproute2.inc
+++ b/meta/recipes-connectivity/iproute2/iproute2.inc
@@ -46,10 +46,11 @@ do_install () {
 # The .so files in iproute2-tc are modules, not traditional libraries
 INSANE_SKIP_${PN}-tc = "dev-so"
 
-PACKAGES =+ "\
+IPROUTE2_PACKAGES =+ "\
     ${PN}-devlink \
     ${PN}-genl \
     ${PN}-ifstat \
+    ${PN}-ip \
     ${PN}-lnstat \
     ${PN}-nstat \
     ${PN}-rtacct \
@@ -58,12 +59,16 @@ PACKAGES =+ "\
     ${PN}-tipc \
 "
 
+PACKAGE_BEFORE_PN = "${IPROUTE2_PACKAGES}"
+RDEPENDS_${PN} += "${IPROUTE2_PACKAGES}"
+
 FILES_${PN}-tc = "${base_sbindir}/tc* \
                   ${libdir}/tc/*.so"
 FILES_${PN}-lnstat = "${base_sbindir}/lnstat \
                       ${base_sbindir}/ctstat \
                       ${base_sbindir}/rtstat"
 FILES_${PN}-ifstat = "${base_sbindir}/ifstat"
+FILES_${PN}-ip = "${base_sbindir}/ip.${PN} ${sysconfdir}/iproute2"
 FILES_${PN}-genl = "${base_sbindir}/genl"
 FILES_${PN}-rtacct = "${base_sbindir}/rtacct"
 FILES_${PN}-nstat = "${base_sbindir}/nstat"
@@ -71,7 +76,7 @@ FILES_${PN}-ss = "${base_sbindir}/ss"
 FILES_${PN}-tipc = "${base_sbindir}/tipc"
 FILES_${PN}-devlink = "${base_sbindir}/devlink"
 
-ALTERNATIVE_${PN} = "ip"
+ALTERNATIVE_${PN}-ip = "ip"
 ALTERNATIVE_TARGET[ip] = "${base_sbindir}/ip.${BPN}"
 ALTERNATIVE_LINK_NAME[ip] = "${base_sbindir}/ip"
 ALTERNATIVE_PRIORITY = "100"
-- 
2.17.1

Re: [OE-core] [meta-oe][PATCH v4] iproute2: split ip to individual package

[Peter Kjellerstedt]

> -----Original Message-----
> From: openembedded-core@lists.openembedded.org <openembedded-
> core@lists.openembedded.org> On Behalf Of Sinan Kaya
> Sent: den 16 december 2020 18:50
> To: openembedded-core@lists.openembedded.org
> Cc: Sinan Kaya <okaya@kernel.org>
> Subject: [OE-core] [meta-oe][PATCH v4] iproute2: split ip to individual
> package
> 
> Move the ip tool into its own package. Useful for size constrained
> systems that only want the ip tool.
> 
> Signed-off-by: Sinan Kaya <okaya@kernel.org>
> ---
>  meta/recipes-connectivity/iproute2/iproute2.inc | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/meta/recipes-connectivity/iproute2/iproute2.inc
> b/meta/recipes-connectivity/iproute2/iproute2.inc
> index 403d264308c..44f336bd580 100644
> --- a/meta/recipes-connectivity/iproute2/iproute2.inc
> +++ b/meta/recipes-connectivity/iproute2/iproute2.inc
> @@ -46,10 +46,11 @@ do_install () {
>  # The .so files in iproute2-tc are modules, not traditional libraries
>  INSANE_SKIP_${PN}-tc = "dev-so"
> 
> -PACKAGES =+ "\
> +IPROUTE2_PACKAGES =+ "\
>      ${PN}-devlink \
>      ${PN}-genl \
>      ${PN}-ifstat \
> +    ${PN}-ip \
>      ${PN}-lnstat \
>      ${PN}-nstat \
>      ${PN}-rtacct \
> @@ -58,12 +59,16 @@ PACKAGES =+ "\
>      ${PN}-tipc \
>  "
> 
> +PACKAGE_BEFORE_PN = "${IPROUTE2_PACKAGES}"
> +RDEPENDS_${PN} += "${IPROUTE2_PACKAGES}"

This breaks backwards compatibility. The iproute2 package today 
does not bring in all the sub-packages. 

> +
>  FILES_${PN}-tc = "${base_sbindir}/tc* \
>                    ${libdir}/tc/*.so"
>  FILES_${PN}-lnstat = "${base_sbindir}/lnstat \
>                        ${base_sbindir}/ctstat \
>                        ${base_sbindir}/rtstat"
>  FILES_${PN}-ifstat = "${base_sbindir}/ifstat"
> +FILES_${PN}-ip = "${base_sbindir}/ip.${PN} ${sysconfdir}/iproute2"
>  FILES_${PN}-genl = "${base_sbindir}/genl"
>  FILES_${PN}-rtacct = "${base_sbindir}/rtacct"
>  FILES_${PN}-nstat = "${base_sbindir}/nstat"
> @@ -71,7 +76,7 @@ FILES_${PN}-ss = "${base_sbindir}/ss"
>  FILES_${PN}-tipc = "${base_sbindir}/tipc"
>  FILES_${PN}-devlink = "${base_sbindir}/devlink"
> 
> -ALTERNATIVE_${PN} = "ip"
> +ALTERNATIVE_${PN}-ip = "ip"
>  ALTERNATIVE_TARGET[ip] = "${base_sbindir}/ip.${BPN}"
>  ALTERNATIVE_LINK_NAME[ip] = "${base_sbindir}/ip"
>  ALTERNATIVE_PRIORITY = "100"
> --
> 2.17.1

If you are this space constrained, I would suggest adding bbappend files 
to your own layer for these recipes instead. That is what we do so we can 
control exactly what files we want or not. E.g., this is what we have in 
our iproute2_%.bbappend:

EXTRA_OEMAKE += "SUBDIRS='lib tc ip'"

do_install_append() {
        # Remove unused files
        rm -f ${D}${base_sbindir}/ifcfg
        rm -f ${D}${base_sbindir}/rtmon
        rm -f ${D}${base_sbindir}/routef
        rm -f ${D}${base_sbindir}/routel
        rm -f ${D}${base_sbindir}/rtpr
}

With the above bbappend, the only binary left in the iproute2 package is in 
fact "ip".

//Peter

Re: [OE-core] [meta-oe][PATCH v4] iproute2: split ip to individual package

[Sinan Kaya]

On 12/16/2020 6:19 PM, Peter Kjellerstedt wrote:
>>
>> -PACKAGES =+ "\
>> +IPROUTE2_PACKAGES =+ "\
>>      ${PN}-devlink \
>>      ${PN}-genl \
>>      ${PN}-ifstat \
>> +    ${PN}-ip \
>>      ${PN}-lnstat \
>>      ${PN}-nstat \
>>      ${PN}-rtacct \
>> @@ -58,12 +59,16 @@ PACKAGES =+ "\
>>      ${PN}-tipc \
>>  "
>>
>> +PACKAGE_BEFORE_PN = "${IPROUTE2_PACKAGES}"
>> +RDEPENDS_${PN} += "${IPROUTE2_PACKAGES}"
> 
> This breaks backwards compatibility. The iproute2 package today 
> does not bring in all the sub-packages. 
> 

OK, Let me fix this.

>> +
>>  FILES_${PN}-tc = "${base_sbindir}/tc* \
>>                    ${libdir}/tc/*.so"
>>  FILES_${PN}-lnstat = "${base_sbindir}/lnstat \
>>                        ${base_sbindir}/ctstat \
>>                        ${base_sbindir}/rtstat"
>>  FILES_${PN}-ifstat = "${base_sbindir}/ifstat"
>> +FILES_${PN}-ip = "${base_sbindir}/ip.${PN} ${sysconfdir}/iproute2"
>>  FILES_${PN}-genl = "${base_sbindir}/genl"
>>  FILES_${PN}-rtacct = "${base_sbindir}/rtacct"
>>  FILES_${PN}-nstat = "${base_sbindir}/nstat"
>> @@ -71,7 +76,7 @@ FILES_${PN}-ss = "${base_sbindir}/ss"
>>  FILES_${PN}-tipc = "${base_sbindir}/tipc"
>>  FILES_${PN}-devlink = "${base_sbindir}/devlink"
>>
>> -ALTERNATIVE_${PN} = "ip"
>> +ALTERNATIVE_${PN}-ip = "ip"
>>  ALTERNATIVE_TARGET[ip] = "${base_sbindir}/ip.${BPN}"
>>  ALTERNATIVE_LINK_NAME[ip] = "${base_sbindir}/ip"
>>  ALTERNATIVE_PRIORITY = "100"
>> --
>> 2.17.1
> 
> If you are this space constrained, I would suggest adding bbappend files 
> to your own layer for these recipes instead. That is what we do so we can 
> control exactly what files we want or not. E.g., this is what we have in 
> our iproute2_%.bbappend:
> 
> EXTRA_OEMAKE += "SUBDIRS='lib tc ip'"
> 
> do_install_append() {
>         # Remove unused files
>         rm -f ${D}${base_sbindir}/ifcfg
>         rm -f ${D}${base_sbindir}/rtmon
>         rm -f ${D}${base_sbindir}/routef
>         rm -f ${D}${base_sbindir}/routel
>         rm -f ${D}${base_sbindir}/rtpr
> }
> 
> With the above bbappend, the only binary left in the iproute2 package is in 
> fact "ip".

I do something very similar in my bbappend now. On the other hand, we
have an upstream first principle in the company. I would rather try to
find an upstream friendly solution that works for everybody without
breaking existing users before falling back to bbappend route that I
need to maintain forever.

> 
> //Peter
> 

Re: [OE-core] [meta-oe][PATCH v4] iproute2: split ip to individual package

[Peter Kjellerstedt]

> -----Original Message-----
> From: Sinan Kaya <okaya@kernel.org>
> Sent: den 17 december 2020 01:31
> To: Peter Kjellerstedt <peter.kjellerstedt@axis.com>; openembedded-
> core@lists.openembedded.org
> Subject: Re: [OE-core] [meta-oe][PATCH v4] iproute2: split ip to
> individual package
> 
> On 12/16/2020 6:19 PM, Peter Kjellerstedt wrote:
> >>
> >> -PACKAGES =+ "\
> >> +IPROUTE2_PACKAGES =+ "\
> >>      ${PN}-devlink \
> >>      ${PN}-genl \
> >>      ${PN}-ifstat \
> >> +    ${PN}-ip \
> >>      ${PN}-lnstat \
> >>      ${PN}-nstat \
> >>      ${PN}-rtacct \
> >> @@ -58,12 +59,16 @@ PACKAGES =+ "\
> >>      ${PN}-tipc \
> >>  "
> >>
> >> +PACKAGE_BEFORE_PN = "${IPROUTE2_PACKAGES}"
> >> +RDEPENDS_${PN} += "${IPROUTE2_PACKAGES}"
> >
> > This breaks backwards compatibility. The iproute2 package today
> > does not bring in all the sub-packages.
> >
> 
> OK, Let me fix this.
> 
> >> +
> >>  FILES_${PN}-tc = "${base_sbindir}/tc* \
> >>                    ${libdir}/tc/*.so"
> >>  FILES_${PN}-lnstat = "${base_sbindir}/lnstat \
> >>                        ${base_sbindir}/ctstat \
> >>                        ${base_sbindir}/rtstat"
> >>  FILES_${PN}-ifstat = "${base_sbindir}/ifstat"
> >> +FILES_${PN}-ip = "${base_sbindir}/ip.${PN} ${sysconfdir}/iproute2"
> >>  FILES_${PN}-genl = "${base_sbindir}/genl"
> >>  FILES_${PN}-rtacct = "${base_sbindir}/rtacct"
> >>  FILES_${PN}-nstat = "${base_sbindir}/nstat"
> >> @@ -71,7 +76,7 @@ FILES_${PN}-ss = "${base_sbindir}/ss"
> >>  FILES_${PN}-tipc = "${base_sbindir}/tipc"
> >>  FILES_${PN}-devlink = "${base_sbindir}/devlink"
> >>
> >> -ALTERNATIVE_${PN} = "ip"
> >> +ALTERNATIVE_${PN}-ip = "ip"
> >>  ALTERNATIVE_TARGET[ip] = "${base_sbindir}/ip.${BPN}"
> >>  ALTERNATIVE_LINK_NAME[ip] = "${base_sbindir}/ip"
> >>  ALTERNATIVE_PRIORITY = "100"
> >> --
> >> 2.17.1
> >
> > If you are this space constrained, I would suggest adding bbappend
> files
> > to your own layer for these recipes instead. That is what we do so we
> can
> > control exactly what files we want or not. E.g., this is what we have
> in
> > our iproute2_%.bbappend:
> >
> > EXTRA_OEMAKE += "SUBDIRS='lib tc ip'"
> >
> > do_install_append() {
> >         # Remove unused files
> >         rm -f ${D}${base_sbindir}/ifcfg
> >         rm -f ${D}${base_sbindir}/rtmon
> >         rm -f ${D}${base_sbindir}/routef
> >         rm -f ${D}${base_sbindir}/routel
> >         rm -f ${D}${base_sbindir}/rtpr
> > }
> >
> > With the above bbappend, the only binary left in the iproute2 package
> is in
> > fact "ip".
> 
> I do something very similar in my bbappend now. On the other hand, we
> have an upstream first principle in the company. I would rather try to
> find an upstream friendly solution that works for everybody without
> breaking existing users before falling back to bbappend route that I
> need to maintain forever.

Well, the problem as I see it is that some of these changes are pretty 
invasive to the recipes. And when most seem fine with them as they are 
(based on the fact that there has not been any push to split any of 
them before AFAIK), the value of these changes are questionable, given 
that more complicated recipes increase the maintenance burden. 

> > //Peter

//Peter

Re: [OE-core] [meta-oe][PATCH v4] iproute2: split ip to individual package

[Sinan Kaya]

On 12/16/2020 8:20 PM, Peter Kjellerstedt wrote:
>> I do something very similar in my bbappend now. On the other hand, we
>> have an upstream first principle in the company. I would rather try to
>> find an upstream friendly solution that works for everybody without
>> breaking existing users before falling back to bbappend route that I
>> need to maintain forever.
> Well, the problem as I see it is that some of these changes are pretty 
> invasive to the recipes. And when most seem fine with them as they are 
> (based on the fact that there has not been any push to split any of 
> them before AFAIK), the value of these changes are questionable, given 
> that more complicated recipes increase the maintenance burden. 
> 

The counter argument is that a user should not be required to rework a
recipe in bbappend for common tools that everybody uses the same way.

If I was doing something special for my target, it has no business in
upstream recipe.

Requesting to have the ip tool out of iproute2 package is a no-brainer
IMO and should be supported by default.

I can also go ahead and say that ip tool probably is the most important
tool in this package and is actually disappointing to see that it has
not been brought out either by PACKAGECONFIG or PACKAGE options. I
honestly don't care about the rest of the tools in that package.

At the end of the day, there is room for improvement in the recipe.
Whether we can do it safely or not is for reviewers to help here.