Re: [OE-core] [PATCH 23/61] powertop: update 2.13 -> 2.14

[Khem Raj]

On Wed, May 5, 2021 at 3:21 PM Alexander Kanavin 
wrote:

> On Thu, 6 May 2021 at 00:02, Khem Raj  wrote:
>
>>
>> order is, look for ncursesw first so this sed operation removes ncursesw
>> from search paths.
>> and this commits mentions this change was intentional
>>
>> https://git.openembedded.org/openembedded-core/commit/?id=188862de64b
>>
>> so why should it be dropped without understanding the repercussions of it?
>>
>
> PKG_CHECK_MODULES does not check for things in order listed (first second
> or whatever), it checks for all of them as a set, or it errors out
> otherwise if anything in the set is not available.
>

Right and it means if widechars support in ncurses is disabled then it
won’t build I know that defaults is to enable it in ncurses so autobuilder
won’t see such failures but many minimal distro did explicitly disable it
in past



>
> Besides, you're asking the question backwards. It should be: why should we
> change the upstream choice if we don't understand the repercussions of
> doing that?
>



> Alex
>

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#151371): 
https://lists.openembedded.org/g/openembedded-core/message/151371
Mute This Topic: https://lists.openembedded.org/mt/82607036/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [OE-core] [PATCH 23/61] powertop: update 2.13 -> 2.14

[Alexander Kanavin]

On Thu, 6 May 2021 at 00:02, Khem Raj  wrote:

>
> order is, look for ncursesw first so this sed operation removes ncursesw
> from search paths.
> and this commits mentions this change was intentional
>
> https://git.openembedded.org/openembedded-core/commit/?id=188862de64b
>
> so why should it be dropped without understanding the repercussions of it?
>

PKG_CHECK_MODULES does not check for things in order listed (first second
or whatever), it checks for all of them as a set, or it errors out
otherwise if anything in the set is not available.

Besides, you're asking the question backwards. It should be: why should we
change the upstream choice if we don't understand the repercussions of
doing that?

Alex

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#151366): 
https://lists.openembedded.org/g/openembedded-core/message/151366
Mute This Topic: https://lists.openembedded.org/mt/82607036/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [OE-core] [PATCH 23/61] powertop: update 2.13 -> 2.14

[Khem Raj]

On Wed, May 5, 2021 at 2:54 PM Alexander Kanavin 
wrote:

> On Wed, 5 May 2021 at 23:26, Khem Raj  wrote:
>
>> I went through the history and some of these changes were added by
>> you, so I hope you know why they were needed
>
>
> None of them are needed to build the new version and therefore none should
> be kept. Sorry, but I do have a strong view on this: the experience with
> various recipes I had to rewrite from scratch because of all the
> intractable, undocumented cruft they accumulated over the years taught me
> this.
>
>
>> # we do not want libncursesw if we can
>> do_configure_prepend() {
>> # configure.ac checks for delwin() in "ncursesw ncurses" so let's
>> drop first one
>> sed -i -e "s/ncursesw//g" ${S}/configure.ac
>> }
>>
>> since we control enabling widechar support in ncurses via
>> ENABLE_WIDEC, its better to either check for that here perhaps via a
>> DISTRO_FEATURE or just disable the widechar support
>> like it is doing.
>>
>
> It's doing:
>
> PKG_CHECK_MODULES([NCURSES], [ncursesw ncurses], [LIBS="$LIBS
> $ncurses_LIBS"], [
> AC_SEARCH_LIBS([delwin], [ncursesw ncurses], [], [
> AC_MSG_ERROR([ncurses is required but was not found])
> ], [])
> ]
>
> Unless you explain why the above is wrong (even though it builds without
> any problem), I'd say you're veering too much towards bikeshedding here.
>
>
order is, look for ncursesw first so this sed operation removes ncursesw
from search paths.
and this commits mentions this change was intentional

https://git.openembedded.org/openembedded-core/commit/?id=188862de64b

so why should it be dropped without understanding the repercussions of it?

Alex
>

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#151362): 
https://lists.openembedded.org/g/openembedded-core/message/151362
Mute This Topic: https://lists.openembedded.org/mt/82607036/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [OE-core] [PATCH 23/61] powertop: update 2.13 -> 2.14

[Alexander Kanavin]

On Wed, 5 May 2021 at 23:26, Khem Raj  wrote:

> I went through the history and some of these changes were added by
> you, so I hope you know why they were needed


None of them are needed to build the new version and therefore none should
be kept. Sorry, but I do have a strong view on this: the experience with
various recipes I had to rewrite from scratch because of all the
intractable, undocumented cruft they accumulated over the years taught me
this.


> # we do not want libncursesw if we can
> do_configure_prepend() {
> # configure.ac checks for delwin() in "ncursesw ncurses" so let's
> drop first one
> sed -i -e "s/ncursesw//g" ${S}/configure.ac
> }
>
> since we control enabling widechar support in ncurses via
> ENABLE_WIDEC, its better to either check for that here perhaps via a
> DISTRO_FEATURE or just disable the widechar support
> like it is doing.
>

It's doing:

PKG_CHECK_MODULES([NCURSES], [ncursesw ncurses], [LIBS="$LIBS $ncurses_LIBS"
], [
AC_SEARCH_LIBS([delwin], [ncursesw ncurses], [], [
AC_MSG_ERROR([ncurses is required but was not found])
], [])
]

Unless you explain why the above is wrong (even though it builds without
any problem), I'd say you're veering too much towards bikeshedding here.

Alex

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#151361): 
https://lists.openembedded.org/g/openembedded-core/message/151361
Mute This Topic: https://lists.openembedded.org/mt/82607036/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [OE-core] [PATCH 23/61] powertop: update 2.13 -> 2.14

[Khem Raj]

On Wed, May 5, 2021 at 12:49 PM Alexander Kanavin
 wrote:
>
> On Wed, 5 May 2021 at 21:43, Khem Raj  wrote:
>>
>> > Sorry, no. If I do not understand what it does, the version update is
>> > incompatible with it,
>> > and there is no trouble detected by the AB from dropping it, then it has
>> > to go.
>>
>> OK thats fair, no one knows everything so drop this patch from your v2
>> series and let someone else handle it.
>
>
> I assure you, nobody else will do it. I regularly drop poorly written things 
> that are a maintenance headache, if you have specific objections or use cases 
> or issue reproducers, please state them, but saying 'don't remove because it 
> might be useful to somebody, somewhere' is far too vague.

I went through the history and some of these changes were added by
you, so I hope you know why they were needed

188862de64b meta/recipes-kernel/powertop/powertop_2.3.bb   (Marcin
Juszkiewicz2013-04-11 16:05:14 +0200 19) do_configure_prepend() {
188862de64b meta/recipes-kernel/powertop/powertop_2.3.bb   (Marcin
Juszkiewicz2013-04-11 16:05:14 +0200 20) # configure.ac checks
for delwin() in "ncursesw ncurses" so let's drop first one
188862de64b meta/recipes-kernel/powertop/powertop_2.3.bb   (Marcin
Juszkiewicz2013-04-11 16:05:14 +0200 21) sed -i -e
"s/ncursesw//g" ${S}/configure.ac
6612c5e48c9 meta/recipes-kernel/powertop/powertop_2.6.1.bb (Richard
Purdie2014-07-16 10:58:34 + 22) mkdir -p
${B}/src/tuning/
bcd6475928d meta/recipes-kernel/powertop/powertop_2.10.bb  (Alexander
Kanavin 2020-05-05 15:01:21 +0200 23) echo "${PV}" >
${S}/version-long
bcd6475928d meta/recipes-kernel/powertop/powertop_2.10.bb  (Alexander
Kanavin 2020-05-05 15:01:21 +0200 24) echo "${PV}" >
${S}/version-short
f1cb1f5b29a meta/recipes-kernel/powertop/powertop_2.12.bb  (Alexander
Kanavin 2020-05-31 17:52:20 +0200 25) cp
${STAGING_DATADIR}/aclocal/ax_require_defined.m4 ${S}/m4/
188862de64b meta/recipes-kernel/powertop/powertop_2.3.bb   (Marcin
Juszkiewicz2013-04-11 16:05:14 +0200 26) }

besidest, I think we still need

# we do not want libncursesw if we can
do_configure_prepend() {
# configure.ac checks for delwin() in "ncursesw ncurses" so let's
drop first one
sed -i -e "s/ncursesw//g" ${S}/configure.ac
}

since we control enabling widechar support in ncurses via
ENABLE_WIDEC, its better to either check for that here perhaps via a
DISTRO_FEATURE or just disable the widechar support
like it is doing.

other perhaps are fine to drop.

>
> Alex

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#151357): 
https://lists.openembedded.org/g/openembedded-core/message/151357
Mute This Topic: https://lists.openembedded.org/mt/82607036/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [OE-core] [PATCH 23/61] powertop: update 2.13 -> 2.14

[Alexander Kanavin]

On Wed, 5 May 2021 at 21:43, Khem Raj  wrote:

> > Sorry, no. If I do not understand what it does, the version update is
> > incompatible with it,
> > and there is no trouble detected by the AB from dropping it, then it has
> > to go.
>
> OK thats fair, no one knows everything so drop this patch from your v2
> series and let someone else handle it.
>

I assure you, nobody else will do it. I regularly drop poorly written
things that are a maintenance headache, if you have specific objections or
use cases or issue reproducers, please state them, but saying 'don't remove
because it might be useful to somebody, somewhere' is far too vague.

Alex

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#151353): 
https://lists.openembedded.org/g/openembedded-core/message/151353
Mute This Topic: https://lists.openembedded.org/mt/82607036/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [OE-core] [PATCH 23/61] powertop: update 2.13 -> 2.14

[Khem Raj]

On 5/5/21 11:52 AM, Alexander Kanavin wrote:
On Wed, 5 May 2021 at 20:13, Khem Raj > wrote:


On Wed, May 5, 2021 at 8:20 AM Alexander Kanavin
mailto:alex.kana...@gmail.com>> wrote:
 >
 > Drop awful ncurses hack; please make a proper patch with a configure
 > option and PACKAGECONFIG.
 >

Lets not delete this unless its causing trouble as it will break
existing functionality that users might be depending upon.
if there is better fix please propose that instead


Sorry, no. If I do not understand what it does, the version update is 
incompatible with it,
and there is no trouble detected by the AB from dropping it, then it has 
to go.


OK thats fair, no one knows everything so drop this patch from your v2 
series and let someone else handle it.




Alex

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#151352): 
https://lists.openembedded.org/g/openembedded-core/message/151352
Mute This Topic: https://lists.openembedded.org/mt/82607036/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [OE-core] [PATCH 23/61] powertop: update 2.13 -> 2.14

[Alexander Kanavin]

On Wed, 5 May 2021 at 20:13, Khem Raj  wrote:

> On Wed, May 5, 2021 at 8:20 AM Alexander Kanavin 
> wrote:
> >
> > Drop awful ncurses hack; please make a proper patch with a configure
> > option and PACKAGECONFIG.
> >
>
> Lets not delete this unless its causing trouble as it will break
> existing functionality that users might be depending upon.
> if there is better fix please propose that instead
>

Sorry, no. If I do not understand what it does, the version update is
incompatible with it,
and there is no trouble detected by the AB from dropping it, then it has to
go.

Alex

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#151343): 
https://lists.openembedded.org/g/openembedded-core/message/151343
Mute This Topic: https://lists.openembedded.org/mt/82607036/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [OE-core] [PATCH 23/61] powertop: update 2.13 -> 2.14

[Khem Raj]

On Wed, May 5, 2021 at 8:20 AM Alexander Kanavin  wrote:
>
> Drop awful ncurses hack; please make a proper patch with a configure
> option and PACKAGECONFIG.
>

Lets not delete this unless its causing trouble as it will break
existing functionality that users might be depending upon.
if there is better fix please propose that instead

> Adjust LDFLAGS as somehow -pthread isn't there --> link failures.
>
> Signed-off-by: Alexander Kanavin 
> ---
>  .../{powertop_2.13.bb => powertop_2.14.bb} | 14 +++---
>  1 file changed, 3 insertions(+), 11 deletions(-)
>  rename meta/recipes-kernel/powertop/{powertop_2.13.bb => powertop_2.14.bb} 
> (64%)
>
> diff --git a/meta/recipes-kernel/powertop/powertop_2.13.bb 
> b/meta/recipes-kernel/powertop/powertop_2.14.bb
> similarity index 64%
> rename from meta/recipes-kernel/powertop/powertop_2.13.bb
> rename to meta/recipes-kernel/powertop/powertop_2.14.bb
> index 8c7e78fd94..cb7f3c4dc1 100644
> --- a/meta/recipes-kernel/powertop/powertop_2.13.bb
> +++ b/meta/recipes-kernel/powertop/powertop_2.14.bb
> @@ -9,21 +9,13 @@ LIC_FILES_CHKSUM = 
> "file://COPYING;md5=12f884d2ae1ff87c09e5b7ccc2c4ca7e"
>  SRC_URI = "git://github.com/fenrus75/powertop;protocol=https \
>  file://0001-wakeup_xxx.h-include-limits.h.patch \
>  "
> -SRCREV = "184cee06b2d64679bae5f806fe0a218827fdde99"
> +SRCREV = "52f022f9bbe6e060fba11701d657a8d9762702ba"
>
>  S = "${WORKDIR}/git"
>
> -inherit autotools gettext pkgconfig bash-completion
> +LDFLAGS_append = " -pthread"
>
> -# we do not want libncursesw if we can
> -do_configure_prepend() {
> -# configure.ac checks for delwin() in "ncursesw ncurses" so let's drop 
> first one
> -sed -i -e "s/ncursesw//g" ${S}/configure.ac
> -mkdir -p ${B}/src/tuning/
> -echo "${PV}" > ${S}/version-long
> -echo "${PV}" > ${S}/version-short
> -cp ${STAGING_DATADIR}/aclocal/ax_require_defined.m4 ${S}/m4/
> -}
> +inherit autotools gettext pkgconfig bash-completion
>
>  inherit update-alternatives
>  ALTERNATIVE_${PN} = "powertop"
> --
> 2.31.1
>
>
> 
>

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#151337): 
https://lists.openembedded.org/g/openembedded-core/message/151337
Mute This Topic: https://lists.openembedded.org/mt/82607036/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-