[wic][PATCH] wic: extended list of paths in find_binary_path

[wic][PATCH] wic: extended list of paths in find_binary_path

[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.

[YOCTO #7122]

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 scripts/lib/wic/utils/fs_related.py | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/scripts/lib/wic/utils/fs_related.py b/scripts/lib/wic/utils/fs_related.py
index ea9f85c..832a44a 100644
--- a/scripts/lib/wic/utils/fs_related.py
+++ b/scripts/lib/wic/utils/fs_related.py
@@ -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)
-- 
2.1.4

Re: [wic][PATCH] wic: extended list of paths in find_binary_path

[Otavio Salvador]

[-- Attachment #1: Type: text/plain, Size: 889 bytes --]

Em 04/04/2015 14:21, "Ed Bartosh" <ed.bartosh@linux.intel.com> escreveu:
>
> 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]
>
> Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>

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.

[-- Attachment #2: Type: text/html, Size: 1171 bytes --]

Re: [wic][PATCH] wic: extended list of paths in find_binary_path

[Ed Bartosh]

On Sat, Apr 04, 2015 at 03:34:59PM -0300, Otavio Salvador wrote:
> Em 04/04/2015 14:21, "Ed Bartosh" <ed.bartosh@linux.intel.com> escreveu:
> >
> > 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]
> >
> > Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
> 
> 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.

> 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

Re: [wic][PATCH] wic: extended list of paths in find_binary_path

[Philip Balister]

On 04/04/2015 02:34 PM, Ed Bartosh wrote:
> On Sat, Apr 04, 2015 at 03:34:59PM -0300, Otavio Salvador wrote:
>> Em 04/04/2015 14:21, "Ed Bartosh" <ed.bartosh@linux.intel.com> escreveu:
>>>
>>> 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]
>>>
>>> Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
>>
>> 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.
> 
>> 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?

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

> 
> --
> Regards,
> Ed
> 

[wic][PATCH] wic: use native parted

[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]

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 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(-)

diff --git a/scripts/lib/wic/imager/direct.py b/scripts/lib/wic/imager/direct.py
index d368401..c3d0563 100644
--- a/scripts/lib/wic/imager/direct.py
+++ b/scripts/lib/wic/imager/direct.py
@@ -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
diff --git a/scripts/lib/wic/utils/fs_related.py b/scripts/lib/wic/utils/fs_related.py
index ea9f85c..033acc9 100644
--- a/scripts/lib/wic/utils/fs_related.py
+++ b/scripts/lib/wic/utils/fs_related.py
@@ -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.
diff --git a/scripts/lib/wic/utils/partitionedfs.py b/scripts/lib/wic/utils/partitionedfs.py
index 162f8e1..1c10cb5 100644
--- a/scripts/lib/wic/utils/partitionedfs.py
+++ b/scripts/lib/wic/utils/partitionedfs.py
@@ -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)
 
-- 
2.1.4

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

[Otavio Salvador]

On Mon, Apr 6, 2015 at 2:43 PM, Ed Bartosh <ed.bartosh@linux.intel.com> wrote:
> 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]
>
> Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>

Awesome!

Acked-by: Otavio Salvador <otavio@ossystems.com.br>

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

-- 
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

Re: [wic][PATCH] wic: extended list of paths in find_binary_path

[Philip Balister]

On 04/04/2015 10:20 AM, Ed Bartosh wrote:
> 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

> 
> [YOCTO #7122]
> 
> Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
> ---
>  scripts/lib/wic/utils/fs_related.py | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/scripts/lib/wic/utils/fs_related.py b/scripts/lib/wic/utils/fs_related.py
> index ea9f85c..832a44a 100644
> --- a/scripts/lib/wic/utils/fs_related.py
> +++ b/scripts/lib/wic/utils/fs_related.py
> @@ -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)
> 

Re: [wic][PATCH] wic: extended list of paths in find_binary_path

[Otavio Salvador]

On Sun, Apr 5, 2015 at 4:16 PM, Philip Balister <philip@balister.org> wrote:
> On 04/04/2015 10:20 AM, Ed Bartosh wrote:
>> 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.

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?).

-- 
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

Re: [wic][PATCH] wic: extended list of paths in find_binary_path

[Ed Bartosh]

On Mon, Apr 06, 2015 at 09:00:50AM -0300, Otavio Salvador wrote:
> On Sun, Apr 5, 2015 at 4:16 PM, Philip Balister <philip@balister.org> wrote:
> > On 04/04/2015 10:20 AM, Ed Bartosh wrote:
> >> 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.
> 
> 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

Re: [wic][PATCH] wic: extended list of paths in find_binary_path

[Otavio Salvador]

On Mon, Apr 6, 2015 at 11:15 AM, Ed Bartosh <ed.bartosh@linux.intel.com> wrote:
> On Mon, Apr 06, 2015 at 09:00:50AM -0300, Otavio Salvador wrote:
>> On Sun, Apr 5, 2015 at 4:16 PM, Philip Balister <philip@balister.org> wrote:
>> > On 04/04/2015 10:20 AM, Ed Bartosh wrote:
>> >> 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.
>>
>> 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.

> 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

> 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.

-- 
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

Re: [wic][PATCH] wic: extended list of paths in find_binary_path

[Ed Bartosh]

On Mon, Apr 06, 2015 at 11:28:24AM -0300, Otavio Salvador wrote:
> >>
> >> 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

Re: [wic][PATCH] wic: extended list of paths in find_binary_path

[Otavio Salvador]

On Tue, Apr 7, 2015 at 7:52 AM, Ed Bartosh <ed.bartosh@linux.intel.com> wrote:
> On Mon, Apr 06, 2015 at 11:28:24AM -0300, Otavio Salvador wrote:
>> >>
>> >> 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.

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).

-- 
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