[thud][PATCH] Revert "OpkgPM: use --add-ignore-recommends to process BAD_RECOMMENDATIONS"

[thud][PATCH] Revert "OpkgPM: use --add-ignore-recommends to process BAD_RECOMMENDATIONS"

[Armin Kuster]

This reverts commit e8cd30ba6cec854d85c7ad47edc208107858a5d7.

This backport introduced an issue not seen the AB QA.

Issue can be seen if
BAD_RECOMMENDATIONS_append = " udev-hwdb" is used
---
 meta/lib/oe/package_manager.py | 41 +++++++++++++++++++++++++++++++++++++++--
 meta/lib/oe/rootfs.py          |  2 ++
 2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/meta/lib/oe/package_manager.py b/meta/lib/oe/package_manager.py
index 7d88048..882e7c4 100644
--- a/meta/lib/oe/package_manager.py
+++ b/meta/lib/oe/package_manager.py
@@ -1329,8 +1329,6 @@ class OpkgPM(OpkgDpkgPM):
         cmd = "%s %s" % (self.opkg_cmd, self.opkg_args)
         for exclude in (self.d.getVar("PACKAGE_EXCLUDE") or "").split():
             cmd += " --add-exclude %s" % exclude
-        for bad_recommendation in (self.d.getVar("BAD_RECOMMENDATIONS") or "").split():
-            cmd += " --add-ignore-recommends %s" % bad_recommendation
         cmd += " install "
         cmd += " ".join(pkgs)
 
@@ -1399,6 +1397,45 @@ class OpkgPM(OpkgDpkgPM):
     def list_installed(self):
         return OpkgPkgsList(self.d, self.target_rootfs, self.config_file).list_pkgs()
 
+    def handle_bad_recommendations(self):
+        bad_recommendations = self.d.getVar("BAD_RECOMMENDATIONS") or ""
+        if bad_recommendations.strip() == "":
+            return
+
+        status_file = os.path.join(self.opkg_dir, "status")
+
+        # If status file existed, it means the bad recommendations has already
+        # been handled
+        if os.path.exists(status_file):
+            return
+
+        cmd = "%s %s info " % (self.opkg_cmd, self.opkg_args)
+
+        with open(status_file, "w+") as status:
+            for pkg in bad_recommendations.split():
+                pkg_info = cmd + pkg
+
+                try:
+                    output = subprocess.check_output(pkg_info.split(), stderr=subprocess.STDOUT).strip().decode("utf-8")
+                except subprocess.CalledProcessError as e:
+                    bb.fatal("Cannot get package info. Command '%s' "
+                             "returned %d:\n%s" % (pkg_info, e.returncode, e.output.decode("utf-8")))
+
+                if output == "":
+                    bb.note("Ignored bad recommendation: '%s' is "
+                            "not a package" % pkg)
+                    continue
+
+                for line in output.split('\n'):
+                    if line.startswith("Status:"):
+                        status.write("Status: deinstall hold not-installed\n")
+                    else:
+                        status.write(line + "\n")
+
+                # Append a blank line after each package entry to ensure that it
+                # is separated from the following entry
+                status.write("\n")
+
     def dummy_install(self, pkgs):
         """
         The following function dummy installs pkgs and returns the log of output.
diff --git a/meta/lib/oe/rootfs.py b/meta/lib/oe/rootfs.py
index aa9fb2e..e5512d0 100644
--- a/meta/lib/oe/rootfs.py
+++ b/meta/lib/oe/rootfs.py
@@ -879,6 +879,8 @@ class OpkgRootfs(DpkgOpkgRootfs):
 
         self.pm.update()
 
+        self.pm.handle_bad_recommendations()
+
         if self.progress_reporter:
             self.progress_reporter.next_stage()
 
-- 
2.7.4

✗ patchtest: failure for Revert "OpkgPM: use --add-ignore-recommends to process BAD_RECOMMENDATIONS"

[Patchwork]

== Series Details ==

Series: Revert "OpkgPM: use --add-ignore-recommends to process BAD_RECOMMENDATIONS"
Revision: 1
URL   : https://patchwork.openembedded.org/series/20533/
State : failure

== Summary ==


Thank you for submitting this patch series to OpenEmbedded Core. This is
an automated response. Several tests have been executed on the proposed
series by patchtest resulting in the following failures:



* Issue             Errors in your Python code were encountered [test_pylint] 
  Suggested fix    Correct the lines introduced by your patch
  Output           Please, fix the listed issues:
                   meta/lib/oe/package_manager.py does not exist



If you believe any of these test results are incorrect, please reply to the
mailing list (openembedded-core@lists.openembedded.org) raising your concerns.
Otherwise we would appreciate you correcting the issues and submitting a new
version of the patchset if applicable. Please ensure you add/increment the
version number when sending the new version (i.e. [PATCH] -> [PATCH v2] ->
[PATCH v3] -> ...).

---
Guidelines:     https://www.openembedded.org/wiki/Commit_Patch_Message_Guidelines
Test framework: http://git.yoctoproject.org/cgit/cgit.cgi/patchtest
Test suite:     http://git.yoctoproject.org/cgit/cgit.cgi/patchtest-oe

Re: [thud][PATCH] Revert "OpkgPM: use --add-ignore-recommends to process BAD_RECOMMENDATIONS"

[Denys Dmytriyenko]

Thank you for this revert! I've been pulling my hair out past couple days 
figuring out why BAD_RECOMMENDATIONS stopped working for us. This fixed it.

-- 
Denys


On Thu, Oct 17, 2019 at 08:21:40AM -0700, Armin Kuster wrote:
> This reverts commit e8cd30ba6cec854d85c7ad47edc208107858a5d7.
> 
> This backport introduced an issue not seen the AB QA.
> 
> Issue can be seen if
> BAD_RECOMMENDATIONS_append = " udev-hwdb" is used
> ---
>  meta/lib/oe/package_manager.py | 41 +++++++++++++++++++++++++++++++++++++++--
>  meta/lib/oe/rootfs.py          |  2 ++
>  2 files changed, 41 insertions(+), 2 deletions(-)
> 
> diff --git a/meta/lib/oe/package_manager.py b/meta/lib/oe/package_manager.py
> index 7d88048..882e7c4 100644
> --- a/meta/lib/oe/package_manager.py
> +++ b/meta/lib/oe/package_manager.py
> @@ -1329,8 +1329,6 @@ class OpkgPM(OpkgDpkgPM):
>          cmd = "%s %s" % (self.opkg_cmd, self.opkg_args)
>          for exclude in (self.d.getVar("PACKAGE_EXCLUDE") or "").split():
>              cmd += " --add-exclude %s" % exclude
> -        for bad_recommendation in (self.d.getVar("BAD_RECOMMENDATIONS") or "").split():
> -            cmd += " --add-ignore-recommends %s" % bad_recommendation
>          cmd += " install "
>          cmd += " ".join(pkgs)
>  
> @@ -1399,6 +1397,45 @@ class OpkgPM(OpkgDpkgPM):
>      def list_installed(self):
>          return OpkgPkgsList(self.d, self.target_rootfs, self.config_file).list_pkgs()
>  
> +    def handle_bad_recommendations(self):
> +        bad_recommendations = self.d.getVar("BAD_RECOMMENDATIONS") or ""
> +        if bad_recommendations.strip() == "":
> +            return
> +
> +        status_file = os.path.join(self.opkg_dir, "status")
> +
> +        # If status file existed, it means the bad recommendations has already
> +        # been handled
> +        if os.path.exists(status_file):
> +            return
> +
> +        cmd = "%s %s info " % (self.opkg_cmd, self.opkg_args)
> +
> +        with open(status_file, "w+") as status:
> +            for pkg in bad_recommendations.split():
> +                pkg_info = cmd + pkg
> +
> +                try:
> +                    output = subprocess.check_output(pkg_info.split(), stderr=subprocess.STDOUT).strip().decode("utf-8")
> +                except subprocess.CalledProcessError as e:
> +                    bb.fatal("Cannot get package info. Command '%s' "
> +                             "returned %d:\n%s" % (pkg_info, e.returncode, e.output.decode("utf-8")))
> +
> +                if output == "":
> +                    bb.note("Ignored bad recommendation: '%s' is "
> +                            "not a package" % pkg)
> +                    continue
> +
> +                for line in output.split('\n'):
> +                    if line.startswith("Status:"):
> +                        status.write("Status: deinstall hold not-installed\n")
> +                    else:
> +                        status.write(line + "\n")
> +
> +                # Append a blank line after each package entry to ensure that it
> +                # is separated from the following entry
> +                status.write("\n")
> +
>      def dummy_install(self, pkgs):
>          """
>          The following function dummy installs pkgs and returns the log of output.
> diff --git a/meta/lib/oe/rootfs.py b/meta/lib/oe/rootfs.py
> index aa9fb2e..e5512d0 100644
> --- a/meta/lib/oe/rootfs.py
> +++ b/meta/lib/oe/rootfs.py
> @@ -879,6 +879,8 @@ class OpkgRootfs(DpkgOpkgRootfs):
>  
>          self.pm.update()
>  
> +        self.pm.handle_bad_recommendations()
> +
>          if self.progress_reporter:
>              self.progress_reporter.next_stage()
>  
> -- 
> 2.7.4
> 
> -- 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [thud][PATCH] Revert "OpkgPM: use --add-ignore-recommends to process BAD_RECOMMENDATIONS"

[akuster808]

On 10/17/19 11:42 AM, Denys Dmytriyenko wrote:
> Thank you for this revert! I've been pulling my hair out past couple days 
> figuring out why BAD_RECOMMENDATIONS stopped working for us. This fixed it.
>

What I find curious is that the offending commit has been in thud since
July 29th and we are just now hearing about issues.

http://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?h=thud&id=e8cd30ba6cec854d85c7ad47edc208107858a5d7

- armin

Re: [thud][PATCH] Revert "OpkgPM: use --add-ignore-recommends to process BAD_RECOMMENDATIONS"

[Denys Dmytriyenko]

On Thu, Oct 17, 2019 at 12:21:11PM -0700, akuster808 wrote:
> On 10/17/19 11:42 AM, Denys Dmytriyenko wrote:
> > Thank you for this revert! I've been pulling my hair out past couple days 
> > figuring out why BAD_RECOMMENDATIONS stopped working for us. This fixed it.
> 
> What I find curious is that the offending commit has been in thud since
> July 29th and we are just now hearing about issues.

Yeah, in our case we only noticed the issue when we started preparing the 
latest release and found extra packages that weren't supposed to be there.

-- 
Denys

Re: [thud][PATCH] Revert "OpkgPM: use --add-ignore-recommends to process BAD_RECOMMENDATIONS"

[Alejandro Del Castillo]

On 10/17/19 3:01 PM, Denys Dmytriyenko wrote:
> On Thu, Oct 17, 2019 at 12:21:11PM -0700, akuster808 wrote:
>> On 10/17/19 11:42 AM, Denys Dmytriyenko wrote:
>>> Thank you for this revert! I've been pulling my hair out past couple days
>>> figuring out why BAD_RECOMMENDATIONS stopped working for us. This fixed it.
>>
>> What I find curious is that the offending commit has been in thud since
>> July 29th and we are just now hearing about issues.
> 
> Yeah, in our case we only noticed the issue when we started preparing the
> latest release and found extra packages that weren't supposed to be there.

Is there a bugzilla ticket tracking the issue? I added the 
--add-ignore-recommends flag to opkg, mainly to get BAD_RECOMMENDATIONS 
to work for OE. If that's not the case, then need to figure out what 
went wrong (the OE side or the opkg side)....

-- 
Cheers,

Alejandro

Re: [thud][PATCH] Revert "OpkgPM: use --add-ignore-recommends to process BAD_RECOMMENDATIONS"

[akuster808]

On 10/18/19 10:58 AM, Alejandro Del Castillo wrote:
>
> On 10/17/19 3:01 PM, Denys Dmytriyenko wrote:
>> On Thu, Oct 17, 2019 at 12:21:11PM -0700, akuster808 wrote:
>>> On 10/17/19 11:42 AM, Denys Dmytriyenko wrote:
>>>> Thank you for this revert! I've been pulling my hair out past couple days
>>>> figuring out why BAD_RECOMMENDATIONS stopped working for us. This fixed it.
>>> What I find curious is that the offending commit has been in thud since
>>> July 29th and we are just now hearing about issues.
>> Yeah, in our case we only noticed the issue when we started preparing the
>> latest release and found extra packages that weren't supposed to be there.
> Is there a bugzilla ticket tracking the issue? 
I am not aware of any for thud. IIRC, this commit was a backport request
from the community.

It would be interesting if those who have issues with thud check against
master.

- armin
> I added the 
> --add-ignore-recommends flag to opkg, mainly to get BAD_RECOMMENDATIONS 
> to work for OE. If that's not the case, then need to figure out what 
> went wrong (the OE side or the opkg side)....
>

Re: [thud][PATCH] Revert "OpkgPM: use --add-ignore-recommends to process BAD_RECOMMENDATIONS"

[Sean Nyekjaer]

On 18/10/2019 23.18, akuster808 wrote:
> I am not aware of any for thud. IIRC, this commit was a backport request
> from the community.
> 
> It would be interesting if those who have issues with thud check against
> master.
> 
> - armin

Hi,

Sorry I was slow to report :)
Was this also an issue on warrior and master?

/Sean

Re: [thud][PATCH] Revert "OpkgPM: use --add-ignore-recommends to process BAD_RECOMMENDATIONS"

[Adrian Bunk]

On Thu, Oct 17, 2019 at 12:21:11PM -0700, akuster808 wrote:
> On 10/17/19 11:42 AM, Denys Dmytriyenko wrote:
> > Thank you for this revert! I've been pulling my hair out past couple days 
> > figuring out why BAD_RECOMMENDATIONS stopped working for us. This fixed it.
> 
> What I find curious is that the offending commit has been in thud since
> July 29th and we are just now hearing about issues.
>...

The "Yocto is not a distribution, it is a tool to build distributions"
adds a layer between Yocto upstream and users that results in users
usually using old Yocto releases.

How large is actually the fraction of users who are creating their own 
distributions based on Yocto releases taken directly from upstream?

I would suspect most users are using 3rd party distributions provided by 
hardware vendors or companies like Wind River.

This adds quite a delay between a change being released in a Yocto 
stable release, and users actually using it.

When the distribution they are using is based on a thud release, new 
Yocto releases from that branch will only reach them when/if their 
distribution provider has updated and validated this change and made
a new release of the distribution.

Related, many of the Yocto user questions in various places tend to be 
for 2-3 year old Yocto releases. Often these sound like people just 
starting a new project with a distibution based on an old Yocto release.

> - armin

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [thud][PATCH] Revert "OpkgPM: use --add-ignore-recommends to process BAD_RECOMMENDATIONS"

[Quentin Schulz]

Hi all,

On Fri, Oct 18, 2019 at 02:18:15PM -0700, akuster808 wrote:
> 
> 
> On 10/18/19 10:58 AM, Alejandro Del Castillo wrote:
> >
> > On 10/17/19 3:01 PM, Denys Dmytriyenko wrote:
> >> On Thu, Oct 17, 2019 at 12:21:11PM -0700, akuster808 wrote:
> >>> On 10/17/19 11:42 AM, Denys Dmytriyenko wrote:
> >>>> Thank you for this revert! I've been pulling my hair out past couple days
> >>>> figuring out why BAD_RECOMMENDATIONS stopped working for us. This fixed it.
> >>> What I find curious is that the offending commit has been in thud since
> >>> July 29th and we are just now hearing about issues.
> >> Yeah, in our case we only noticed the issue when we started preparing the
> >> latest release and found extra packages that weren't supposed to be there.
> > Is there a bugzilla ticket tracking the issue? 
> I am not aware of any for thud. IIRC, this commit was a backport request
> from the community.
> 
> It would be interesting if those who have issues with thud check against
> master.
> 

Given that I took the patches from 2.7 (warrior) for backport, we should
be safe from that side.

Tested anyway with poky c9a1a608f5146e5dcfebc8e566cb7f6388a3986f
(warrior), and:
1. BAD_RECOMMENDATIONS are not pulled, as intended and expected,
2. the bug I explained in my cover letter[1] as a reason for backporting
the patch Armin has since reverted does not happen,

Quentin

[1] https://lists.yoctoproject.org/g/poky/message/11671
-- 
StreamUnlimited Engineering GmbH
High Tech Campus Vienna, Gutheil-Schoder-Gasse 10, 1100 Vienna, Austria
quentin.schulz@streamunlimited.com, www.streamunlimited.com