Skip to contentSkip to conversation

wic: extended list of paths in find_binary_path

PATCH In review Ed Bartosh 3 participants 12 messages Apr 4, 2015 – Apr 7, 2015

lore
Share & tools
lore

Ed Bartosh

author
#0 lore raw

PATCH 1/1 wic: extended list of paths in find_binary_path

wic requires tools that are not always possible to find in $PATH. This causes wic to fail with confusing errors like this:
External command 'parted' not found, exiting.
(Please install 'parted' on your host system)

Adding ~/bin/, /usr/local/sbin, /usr/local/bin, /usr/sbin, /usr/bin, /sbin and /bin to the list of paths makes find_binary_path to produce more reliable results.

[YOCTO #7122]

---
 scripts/lib/wic/utils/fs_related.py | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)
scripts/lib/wic/utils/fs_related.py +8 -5
index ea9f85c..832a44a 100644
@@ -32,13 +32,16 @@ from wic.utils.errors import *
 from wic.utils.oe.misc import *
 
 def find_binary_path(binary):
+    paths = []
     if os.environ.has_key("PATH"):
         paths = os.environ["PATH"].split(":")
-    else:
-        paths = []
-        if os.environ.has_key("HOME"):
-            paths += [os.environ["HOME"] + "/bin"]
-        paths += ["/usr/local/sbin", "/usr/local/bin", "/usr/sbin", "/usr/bin", "/sbin", "/bin"]
+    if os.environ.has_key("HOME"):
+        path = os.path.join(os.environ["HOME"], "bin")
+        if path not in paths:
+            paths.append(path)
+    for path in ["/usr/local/sbin", "/usr/local/bin", "/usr/sbin", "/usr/bin", "/sbin", "/bin"]:
+        if path not in paths:
+            paths.append(path)
 
     for path in paths:
         bin_path = "%s/%s" % (path, binary)
Signature · 1 line
2.1.4
#1 lore raw

Em 04/04/2015 14:21, "Ed Bartosh" <[email protected]> escreveu:

Quoted 6 lines

wic requires tools that are not always possible to find in $PATH. This causes wic to fail with confusing errors like this:
External command 'parted' not found, exiting.
(Please install 'parted' on your host system)

Quoted 7 lines

Adding ~/bin/, /usr/local/sbin, /usr/local/bin, /usr/sbin, /usr/bin, /sbin and /bin to the list of paths makes find_binary_path to produce more reliable results.

[YOCTO #7122]

I think we shouldn't do that. If user wants something to be found the PATH environment variable should have them included.

This also brings one serious error off wic which is it relying on host utilities. It should use parted from native sysroot instead otherwise we cannot be sure if witch version we will be using.

author
#2 lore raw
Quoted 11 lines · Otavio Salvador

Em 04/04/2015 14:21, "Ed Bartosh" <[email protected]> escreveu:

Quoted 10 lines

wic requires tools that are not always possible to find in $PATH. This causes wic to fail with confusing errors like this:
External command 'parted' not found, exiting.
(Please install 'parted' on your host system)

Adding ~/bin/, /usr/local/sbin, /usr/local/bin, /usr/sbin, /usr/bin, /sbin and /bin to the list of paths makes find_binary_path to produce more reliable results.

Quoted 7 lines
Quoted 3 lines

[YOCTO #7122]

I think we shouldn't do that. If user wants something to be found the PATH environment variable should have them included.

Some users think that this is confusing: https://bugzilla.yoctoproject.org/show_bug.cgi?id=7122
For current implementation of wic it practically means that it must be always run as "PATH=/usr/sbin:$PATH wic" on OpenSUSE systems.

Quoted 3 lines

This also brings one serious error off wic which is it relying on host utilities. It should use parted from native sysroot instead otherwise we cannot be sure if witch version we will be using.

This makes sense to me. Not sure how easy is to do it as I suspect that not only parted is used this way.

BTW, this approach conflicts with this feature request: https://bugzilla.yoctoproject.org/show_bug.cgi?id=6558
So, should wic rely on system utilities or not?
If it should not then what would you suggest to do with #6558?

--
Regards,
Ed

#3 lore raw
Quoted 26 lines · Ed Bartosh
Quoted 11 lines · Otavio Salvador

Em 04/04/2015 14:21, "Ed Bartosh" <[email protected]> escreveu:

Quoted 10 lines

wic requires tools that are not always possible to find in $PATH. This causes wic to fail with confusing errors like this:
External command 'parted' not found, exiting.
(Please install 'parted' on your host system)

Adding ~/bin/, /usr/local/sbin, /usr/local/bin, /usr/sbin, /usr/bin, /sbin and /bin to the list of paths makes find_binary_path to produce more reliable results.

Quoted 7 lines
Quoted 3 lines

[YOCTO #7122]

I think we shouldn't do that. If user wants something to be found the PATH environment variable should have them included.

Some users think that this is confusing: https://bugzilla.yoctoproject.org/show_bug.cgi?id=7122
For current implementation of wic it practically means that it must be always run as "PATH=/usr/sbin:$PATH wic" on OpenSUSE systems.

Quoted 3 lines

This also brings one serious error off wic which is it relying on host utilities. It should use parted from native sysroot instead otherwise we cannot be sure if witch version we will be using.

Quoted 5 lines

This makes sense to me. Not sure how easy is to do it as I suspect that not only parted is used this way.

BTW, this approach conflicts with this feature request: https://bugzilla.yoctoproject.org/show_bug.cgi?id=6558
So, should wic rely on system utilities or not?
If it should not then what would you suggest to do with #6558?

I added a comment to this. Basically wic is very dependent on data from the build system. It has no knowledge of packages. If we want to build images outside the build system from packages/package feeds, that would be a different tool than wic.

Philip

Quoted 5 lines

--
Regards,
Ed

author
#4 lore raw

PATCH 1/1 wic: use native parted

In reply to Otavio Salvador

Used exec_native_cmd instead of find_binary_path to run parted. Got rid of find_binary_path as it's not used anywhere else.

There are several tools wic is trying to find not only in sysroot, but also in host root. Parted is a special as on some distros it's installed in /usr/sbin, which is not in the user's PATH. This makes wic to fail with error "External command 'parted' not found, exiting."

[YOCTO #7122]

---
 scripts/lib/wic/imager/direct.py       |  2 +-
 scripts/lib/wic/utils/fs_related.py    | 18 ------------------
 scripts/lib/wic/utils/partitionedfs.py | 11 ++++++-----
 3 files changed, 7 insertions(+), 24 deletions(-)
scripts/lib/wic/imager/direct.py +1 -1
index d368401..c3d0563 100644
@@ -242,7 +242,7 @@ class DirectImageCreator(BaseImageCreator):
         """
         parts = self._get_parts()
 
-        self.__image = Image()
+        self.__image = Image(self.native_sysroot)
 
         for p in parts:
             # as a convenience, set source to the boot partition source
scripts/lib/wic/utils/fs_related.py +0 -18
index ea9f85c..033acc9 100644
@@ -31,24 +31,6 @@ from wic.utils import runner
 from wic.utils.errors import *
 from wic.utils.oe.misc import *
 
-def find_binary_path(binary):
-    if os.environ.has_key("PATH"):
-        paths = os.environ["PATH"].split(":")
-    else:
-        paths = []
-        if os.environ.has_key("HOME"):
-            paths += [os.environ["HOME"] + "/bin"]
-        paths += ["/usr/local/sbin", "/usr/local/bin", "/usr/sbin", "/usr/bin", "/sbin", "/bin"]
-
-    for path in paths:
-        bin_path = "%s/%s" % (path, binary)
-        if os.path.exists(bin_path):
-            return bin_path
-
-    print "External command '%s' not found, exiting." % binary
-    print "  (Please install '%s' on your host system)" % binary
-    sys.exit(1)
-
 def makedirs(dirname):
     """A version of os.makedirs() that doesn't throw an
     exception if the leaf directory already exists.
scripts/lib/wic/utils/partitionedfs.py +6 -5
index 162f8e1..1c10cb5 100644
@@ -42,13 +42,13 @@ class Image:
     An Image is a container for a set of DiskImages and associated
     partitions.
     """
-    def __init__(self):
+    def __init__(self, native_sysroot=None):
         self.disks = {}
         self.partitions = []
-        self.parted = find_binary_path("parted")
         # Size of a sector used in calculations
         self.sector_size = SECTOR_SIZE
         self._partitions_layed_out = False
+        self.native_sysroot = native_sysroot
 
     def __add_disk(self, disk_name):
         """ Add a disk 'disk_name' to the internal list of disks. Note,
@@ -227,11 +227,12 @@ class Image:
     def __run_parted(self, args):
         """ Run parted with arguments specified in the 'args' list. """
 
-        args.insert(0, self.parted)
+        args.insert(0, "parted")
+        args = ' '.join(args)
         msger.debug(args)
 
-        rc, out = runner.runtool(args, catch = 3)
-        out = out.strip()
+        rc, out = exec_native_cmd(args, self.native_sysroot)
+
         if out:
             msger.debug('"parted" output: %s' % out)
 
Signature · 1 line
2.1.4
Acked-by on v1 1/1
#5 lore raw

Re: [wic][PATCH] wic: use native parted

Quoted 11 lines · Ed Bartosh

Used exec_native_cmd instead of find_binary_path to run parted. Got rid of find_binary_path as it's not used anywhere else.

There are several tools wic is trying to find not only in sysroot, but also in host root. Parted is a special as on some distros it's installed in /usr/sbin, which is not in the user's PATH. This makes wic to fail with error "External command 'parted' not found, exiting."

[YOCTO #7122]

Awesome!

Please open a doc update bug as well. This has Doc impact.

Signature · 3 lines
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750
#6 lore raw
In reply to Ed Bartosh
Quoted 8 lines · Ed Bartosh

wic requires tools that are not always possible to find in $PATH. This causes wic to fail with confusing errors like this:
External command 'parted' not found, exiting.
(Please install 'parted' on your host system)

Adding ~/bin/, /usr/local/sbin, /usr/local/bin, /usr/sbin, /usr/bin, /sbin and /bin to the list of paths makes find_binary_path to produce more reliable results.

I'm with Otavio. Given wic is intimately dependent on internal build artifacts, it should use utilities from the sysroot and not depend on tools from the path. Let's make wic 100% before creating a tool to build images from packages.

Philip

Quoted 35 lines

[YOCTO #7122]

---
 scripts/lib/wic/utils/fs_related.py | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)
scripts/lib/wic/utils/fs_related.py +8 -5
index ea9f85c..832a44a 100644
@@ -32,13 +32,16 @@ from wic.utils.errors import *
 from wic.utils.oe.misc import *
 
 def find_binary_path(binary):
+    paths = []
     if os.environ.has_key("PATH"):
         paths = os.environ["PATH"].split(":")
-    else:
-        paths = []
-        if os.environ.has_key("HOME"):
-            paths += [os.environ["HOME"] + "/bin"]
-        paths += ["/usr/local/sbin", "/usr/local/bin", "/usr/sbin", "/usr/bin", "/sbin", "/bin"]
+    if os.environ.has_key("HOME"):
+        path = os.path.join(os.environ["HOME"], "bin")
+        if path not in paths:
+            paths.append(path)
+    for path in ["/usr/local/sbin", "/usr/local/bin", "/usr/sbin", "/usr/bin", "/sbin", "/bin"]:
+        if path not in paths:
+            paths.append(path)
 
     for path in paths:
         bin_path = "%s/%s" % (path, binary)
#7 lore raw
Quoted 6 lines · Philip Balister
Quoted 5 lines · Ed Bartosh

wic requires tools that are not always possible to find in $PATH. This causes wic to fail with confusing errors like this:
External command 'parted' not found, exiting.
(Please install 'parted' on your host system)

Quoted 8 lines
Quoted 3 lines

Adding ~/bin/, /usr/local/sbin, /usr/local/bin, /usr/sbin, /usr/bin, /sbin and /bin to the list of paths makes find_binary_path to produce more reliable results.

I'm with Otavio. Given wic is intimately dependent on internal build artifacts, it should use utilities from the sysroot and not depend on tools from the path. Let's make wic 100% before creating a tool to build images from packages.

If we want to have it as a 'standalone' tool we will need to somehow package the tool set for it to use, otherwise we have a maintenance nightmare (does patch X has been applied to Parted?).

Signature · 3 lines
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750
author
#8 lore raw
Quoted 11 lines · Otavio Salvador
Quoted 10 lines · Philip Balister
Quoted 8 lines · Ed Bartosh

wic requires tools that are not always possible to find in $PATH. This causes wic to fail with confusing errors like this:
External command 'parted' not found, exiting.
(Please install 'parted' on your host system)

Adding ~/bin/, /usr/local/sbin, /usr/local/bin, /usr/sbin, /usr/bin, /sbin and /bin to the list of paths makes find_binary_path to produce more reliable results.

Quoted 9 lines
Quoted 4 lines

I'm with Otavio. Given wic is intimately dependent on internal build artifacts, it should use utilities from the sysroot and not depend on tools from the path. Let's make wic 100% before creating a tool to build images from packages.

If we want to have it as a 'standalone' tool we will need to somehow package the tool set for it to use, otherwise we have a maintenance nightmare (does patch X has been applied to Parted?).

Can you elaborate a bit on how tool set can be packaged?

Back to the original topic. wic never used native parted as it's not useable(there is no executable in systoor). Anybody knows the reason?

Documentation also suggests to install parted on the host: http://www.yoctoproject.org/docs/latest/dev-manual/dev-manual.html#wic-requirements

--
Regards,
Ed

#9 lore raw
Quoted 17 lines · Ed Bartosh
Quoted 16 lines · Otavio Salvador
Quoted 14 lines · Philip Balister
Quoted 8 lines · Ed Bartosh

wic requires tools that are not always possible to find in $PATH. This causes wic to fail with confusing errors like this:
External command 'parted' not found, exiting.
(Please install 'parted' on your host system)

Adding ~/bin/, /usr/local/sbin, /usr/local/bin, /usr/sbin, /usr/bin, /sbin and /bin to the list of paths makes find_binary_path to produce more reliable results.

I'm with Otavio. Given wic is intimately dependent on internal build artifacts, it should use utilities from the sysroot and not depend on tools from the path. Let's make wic 100% before creating a tool to build images from packages.

Quoted 5 lines
Quoted 4 lines

If we want to have it as a 'standalone' tool we will need to somehow package the tool set for it to use, otherwise we have a maintenance nightmare (does patch X has been applied to Parted?).

Can you elaborate a bit on how tool set can be packaged?

We could do a tarball like the buildtools toolchain.

Quoted 2 lines

Back to the original topic. wic never used native parted as it's not useable(there is no executable in systoor). Anybody knows the reason?

bitbake parted-native

Quoted 1 line

Documentation also suggests to install parted on the host: http://www.yoctoproject.org/docs/latest/dev-manual/dev-manual.html#wic-requirements

So it also needs fixing.

Signature · 3 lines
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750
author
#10 lore raw
Quoted 8 lines · Otavio Salvador
Quoted 6 lines
Quoted 5 lines

If we want to have it as a 'standalone' tool we will need to somehow package the tool set for it to use, otherwise we have a maintenance nightmare (does patch X has been applied to Parted?).

Can you elaborate a bit on how tool set can be packaged?

We could do a tarball like the buildtools toolchain.

I was thinking about providing wic and its dependencies as repository with rpm/deb packages. Providing them as a tarball doesn't differ much from what we already have from my point of view.

--
Regards,
Ed

#11 lore raw
Quoted 6 lines · Ed Bartosh
Quoted 5 lines · Otavio Salvador
Quoted 5 lines
Quoted 5 lines

If we want to have it as a 'standalone' tool we will need to somehow package the tool set for it to use, otherwise we have a maintenance nightmare (does patch X has been applied to Parted?).

Quoted 6 lines
Quoted 3 lines
Quoted 1 line

Can you elaborate a bit on how tool set can be packaged?

We could do a tarball like the buildtools toolchain.

I was thinking about providing wic and its dependencies as repository with rpm/deb packages. Providing them as a tarball doesn't differ much from what we already have from my point of view.

There are several solutions for this very same problem. Each one has advantages or disadvantages.

To be honest, I think we ought to focus in making wic more easy to use and robust right now. Later we can think about how to enable side use cases (Buildroot might be one).

Signature · 3 lines
Otavio Salvador                             O.S. Systems
http://www.ossystems.com.br        http://code.ossystems.com.br
Mobile: +55 (53) 9981-7854            Mobile: +1 (347) 903-9750