[PATCH] package_manager: don't search for binaries in $PATH explicitly

[PATCH] package_manager: don't search for binaries in $PATH explicitly

[Ross Burton]

There's no point in looking for a command on $PATH using bb.utils.which() but
then passing it to subprocess.check*() which will search $PATH.

By just using the command directly, the code is visibly neater.

Signed-off-by: Ross Burton <ross.burton@intel.com>
---
 meta/lib/oe/package_manager.py | 43 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/meta/lib/oe/package_manager.py b/meta/lib/oe/package_manager.py
index f08190efc0d..f0d98dd0ffc 100644
--- a/meta/lib/oe/package_manager.py
+++ b/meta/lib/oe/package_manager.py
@@ -155,8 +155,7 @@ class RpmIndexer(Indexer):
         else:
             signer = None
 
-        createrepo_c = bb.utils.which(os.environ['PATH'], "createrepo_c")
-        create_index("%s --update -q %s" % (createrepo_c, deploy_dir))
+        create_index("createrepo_c --update -q %s" % (deploy_dir))
 
         # Sign repomd
         if signer:
@@ -316,11 +315,11 @@ class OpkgPkgsList(PkgsList):
     def __init__(self, d, rootfs_dir, config_file):
         super(OpkgPkgsList, self).__init__(d, rootfs_dir)
 
-        self.opkg_args = "-f %s -o %s " % (config_file, rootfs_dir)
-        self.opkg_args += self.d.getVar("OPKG_ARGS")
+        self.opkg_cmd = "opkg -f %s -o %s " % (config_file, rootfs_dir)
+        self.opkg_cmd += self.d.getVar("OPKG_ARGS")
 
     def list_pkgs(self, format=None):
-        cmd = "opkg %s status" % (self.opkg_args)
+        cmd = "%s status" % (self.opkg_cmd)
 
         # opkg returns success even when it printed some
         # "Collected errors:" report to stderr. Mixing stderr into
@@ -870,12 +869,12 @@ class RpmPM(PackageManager):
             args = ["rpm", "-e", "-v", "--nodeps", "--root=%s" %self.target_rootfs]
 
             try:
-                bb.note("Running %s" % ' '.join([cmd] + args + pkgs))
-                output = subprocess.check_output([cmd] + args + pkgs, stderr=subprocess.STDOUT).decode("utf-8")
+                bb.note("Running %s" % ' '.join(args + pkgs))
+                output = subprocess.check_output(args + pkgs, stderr=subprocess.STDOUT).decode("utf-8")
                 bb.note(output)
             except subprocess.CalledProcessError as e:
                 bb.fatal("Could not invoke rpm. Command "
-                         "'%s' returned %d:\n%s" % (' '.join([cmd] + args + pkgs), e.returncode, e.output.decode("utf-8")))
+                         "'%s' returned %d:\n%s" % (' '.join(args + pkgs), e.returncode, e.output.decode("utf-8")))
 
     def upgrade(self):
         self._prepare_pkg_transaction()
@@ -1132,8 +1131,8 @@ class OpkgPM(OpkgDpkgPM):
 
         self.deploy_dir = oe.path.join(self.d.getVar('WORKDIR'), ipk_repo_workdir)
         self.deploy_lock_file = os.path.join(self.deploy_dir, "deploy.lock")
-        self.opkg_args = "--volatile-cache -f %s -t %s -o %s " % (self.config_file, self.d.expand('${T}/ipktemp/'), target_rootfs)
-        self.opkg_args += self.d.getVar("OPKG_ARGS")
+        self.opkg_cmd = "opkg --volatile-cache -f %s -t %s -o %s " % (self.config_file, self.d.expand('${T}/ipktemp/'), target_rootfs)
+        self.opkg_cmd += self.d.getVar("OPKG_ARGS")
 
         if prepare_index:
             create_packages_dir(self.d, self.deploy_dir, d.getVar("DEPLOY_DIR_IPK"), "package_write_ipk", filterbydependencies)
@@ -1292,7 +1291,7 @@ class OpkgPM(OpkgDpkgPM):
     def update(self):
         self.deploy_dir_lock()
 
-        cmd = "opkg %s update" % (self.opkg_args)
+        cmd = "%s update" % (self.opkg_cmd)
 
         try:
             subprocess.check_output(cmd.split(), stderr=subprocess.STDOUT)
@@ -1307,7 +1306,7 @@ class OpkgPM(OpkgDpkgPM):
         if not pkgs:
             return
 
-        cmd = "opkg %s" % (self.opkg_args)
+        cmd = self.opkg_cmd
         for exclude in (self.d.getVar("PACKAGE_EXCLUDE") or "").split():
             cmd += " --add-exclude %s" % exclude
         cmd += " install "
@@ -1342,11 +1341,11 @@ class OpkgPM(OpkgDpkgPM):
             return
 
         if with_dependencies:
-            cmd = "opkg %s --force-remove --force-removal-of-dependent-packages remove %s" % \
-                (self.opkg_args, ' '.join(pkgs))
+            cmd = "%s --force-remove --force-removal-of-dependent-packages remove %s" % \
+                (self.opkg_cmd, ' '.join(pkgs))
         else:
-            cmd = "opkg %s --force-depends remove %s" % \
-                (self.opkg_args, ' '.join(pkgs))
+            cmd = "%s --force-depends remove %s" % \
+                (self.opkg_cmd, ' '.join(pkgs))
 
         try:
             bb.note(cmd)
@@ -1390,7 +1389,7 @@ class OpkgPM(OpkgDpkgPM):
         if os.path.exists(status_file):
             return
 
-        cmd = "opkg %s info " % (self.opkg_args)
+        cmd = "%s info " % (self.opkg_cmd)
 
         with open(status_file, "w+") as status:
             for pkg in bad_recommendations.split():
@@ -1432,10 +1431,10 @@ class OpkgPM(OpkgDpkgPM):
         temp_opkg_dir = os.path.join(temp_rootfs, opkg_lib_dir, 'opkg')
         bb.utils.mkdirhier(temp_opkg_dir)
 
-        opkg_args = "-f %s -o %s " % (self.config_file, temp_rootfs)
+        opkg_args = "opkg -f %s -o %s " % (self.config_file, temp_rootfs)
         opkg_args += self.d.getVar("OPKG_ARGS")
-
-        cmd = "opkg %s update" % (opkg_args)
+        
+        cmd = "%s update" % (opkg_args)
         try:
             subprocess.check_output(cmd, stderr=subprocess.STDOUT, shell=True)
         except subprocess.CalledProcessError as e:
@@ -1443,7 +1442,7 @@ class OpkgPM(OpkgDpkgPM):
                      "returned %d:\n%s" % (cmd, e.returncode, e.output.decode("utf-8")))
 
         # Dummy installation
-        cmd = "opkg %s --noaction install %s " % (opkg_args, ' '.join(pkgs))
+        cmd = "%s --noaction install %s " % (opkg_args, ' '.join(pkgs))
         try:
             output = subprocess.check_output(cmd, stderr=subprocess.STDOUT, shell=True)
         except subprocess.CalledProcessError as e:
@@ -1477,7 +1476,7 @@ class OpkgPM(OpkgDpkgPM):
         """
         Returns a dictionary with the package info.
         """
-        cmd = "opkg %s info %s" % (self.opkg_args, pkg)
+        cmd = "%s info %s" % (self.opkg_cmd, pkg)
         pkg_info = super(OpkgPM, self).package_info(pkg, cmd)
 
         pkg_arch = pkg_info[pkg]["arch"]
-- 
2.11.0

✗ patchtest: failure for package_manager: don't search for binaries in $PATH explicitly

[Patchwork]

== Series Details ==

Series: package_manager: don't search for binaries in $PATH explicitly
Revision: 1
URL   : https://patchwork.openembedded.org/series/15352/
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             Series does not apply on top of target branch [test_series_merge_on_head] 
  Suggested fix    Rebase your series on top of targeted branch
  Targeted branch  master (currently at 6408b4b908)



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: [PATCH] package_manager: don't search for binaries in $PATH explicitly

[Richard Purdie]

On Fri, 2018-12-14 at 17:10 +0000, Ross Burton wrote:
> There's no point in looking for a command on $PATH using
> bb.utils.which() but
> then passing it to subprocess.check*() which will search $PATH.
> 
> By just using the command directly, the code is visibly neater.
> 
> Signed-off-by: Ross Burton <ross.burton@intel.com>
> ---
>  meta/lib/oe/package_manager.py | 43 +++++++++++++++++++++-----------
> ----------
>  1 file changed, 21 insertions(+), 22 deletions(-)

This needs refreshing once the confusion with it and the runqemu path
is sorted!

Cheers,

Richard