[PATCH] rootfs.py: support absolute paths in IMAGE_DEVICE_TABLES

[PATCH] rootfs.py: support absolute paths in IMAGE_DEVICE_TABLES

[Andre McCurdy]

Paths relative to BBPATH are fine if device table files are always
static and contained somewhere within a meta layer. However if device
tables are created dynamically, they will be located somewhere within
${TMPDIR} and most conveniently referenced via an absolute path.

The legacy IMAGE_DEVICE_TABLE variable supported an absolute path,
therefore make IMAGE_DEVICE_TABLES support absolute paths too, to
avoid users who need dynamic device table files being dependent on
the legacy variable.

Signed-off-by: Andre McCurdy <armccurdy@gmail.com>
---
 meta/lib/oe/rootfs.py | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/meta/lib/oe/rootfs.py b/meta/lib/oe/rootfs.py
index a348b97..32cce64 100644
--- a/meta/lib/oe/rootfs.py
+++ b/meta/lib/oe/rootfs.py
@@ -352,8 +352,8 @@ class Rootfs(object, metaclass=ABCMeta):
     """
     Create devfs:
     * IMAGE_DEVICE_TABLE is the old name to an absolute path to a device table file
-    * IMAGE_DEVICE_TABLES is a new name for a file, or list of files, seached
-      for in the BBPATH
+    * IMAGE_DEVICE_TABLES is the new name for a file, or list of files, which may be
+      either absolute paths or paths relative to BBPATH.
     If neither are specified then the default name of files/device_table-minimal.txt
     is searched for in the BBPATH (same as the old version.)
     """
@@ -367,7 +367,10 @@ class Rootfs(object, metaclass=ABCMeta):
             if devtables is None:
                 devtables = 'files/device_table-minimal.txt'
             for devtable in devtables.split():
-                devtable_list.append("%s" % bb.utils.which(self.d.getVar('BBPATH', True), devtable))
+                if os.path.isabs(devtable):
+                    devtable_list.append(devtable)
+                else:
+                    devtable_list.append("%s" % bb.utils.which(self.d.getVar('BBPATH', True), devtable))
 
         for devtable in devtable_list:
             self._exec_shell_cmd(["makedevs", "-r",
-- 
1.9.1

Re: [PATCH] rootfs.py: support absolute paths in IMAGE_DEVICE_TABLES

[Mark Hatle]

On 9/12/16 6:00 PM, Andre McCurdy wrote:
> Paths relative to BBPATH are fine if device table files are always
> static and contained somewhere within a meta layer. However if device
> tables are created dynamically, they will be located somewhere within
> ${TMPDIR} and most conveniently referenced via an absolute path.
> 
> The legacy IMAGE_DEVICE_TABLE variable supported an absolute path,
> therefore make IMAGE_DEVICE_TABLES support absolute paths too, to
> avoid users who need dynamic device table files being dependent on
> the legacy variable.
> 
> Signed-off-by: Andre McCurdy <armccurdy@gmail.com>
> ---
>  meta/lib/oe/rootfs.py | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/meta/lib/oe/rootfs.py b/meta/lib/oe/rootfs.py
> index a348b97..32cce64 100644
> --- a/meta/lib/oe/rootfs.py
> +++ b/meta/lib/oe/rootfs.py
> @@ -352,8 +352,8 @@ class Rootfs(object, metaclass=ABCMeta):
>      """
>      Create devfs:
>      * IMAGE_DEVICE_TABLE is the old name to an absolute path to a device table file
> -    * IMAGE_DEVICE_TABLES is a new name for a file, or list of files, seached
> -      for in the BBPATH
> +    * IMAGE_DEVICE_TABLES is the new name for a file, or list of files, which may be
> +      either absolute paths or paths relative to BBPATH.
>      If neither are specified then the default name of files/device_table-minimal.txt
>      is searched for in the BBPATH (same as the old version.)
>      """
> @@ -367,7 +367,10 @@ class Rootfs(object, metaclass=ABCMeta):
>              if devtables is None:
>                  devtables = 'files/device_table-minimal.txt'
>              for devtable in devtables.split():
> -                devtable_list.append("%s" % bb.utils.which(self.d.getVar('BBPATH', True), devtable))
> +                if os.path.isabs(devtable):
> +                    devtable_list.append(devtable)
> +                else:
> +                    devtable_list.append("%s" % bb.utils.which(self.d.getVar('BBPATH', True), devtable))

I'm wondering if maybe this is a bug in "bb.utils.which", I thought it would
return back -- in the case of an absolute path -- if the thing existed or not,
otherwise would search the BBPATH.  (I'm using the command line version of
'which' as my behavioral thinking.)

If it is an issue, then it would need a fix in bitbake, and I believe the
existing code should then work.

(Asking about the behavior on the bitbake list might be worth while in this case.)

>  
>          for devtable in devtable_list:
>              self._exec_shell_cmd(["makedevs", "-r",
> 

Re: [PATCH] rootfs.py: support absolute paths in IMAGE_DEVICE_TABLES

[Andre McCurdy]

On Mon, Sep 12, 2016 at 5:17 PM, Mark Hatle <mark.hatle@windriver.com> wrote:
> On 9/12/16 6:00 PM, Andre McCurdy wrote:
>> Paths relative to BBPATH are fine if device table files are always
>> static and contained somewhere within a meta layer. However if device
>> tables are created dynamically, they will be located somewhere within
>> ${TMPDIR} and most conveniently referenced via an absolute path.
>>
>> The legacy IMAGE_DEVICE_TABLE variable supported an absolute path,
>> therefore make IMAGE_DEVICE_TABLES support absolute paths too, to
>> avoid users who need dynamic device table files being dependent on
>> the legacy variable.
>>
>> Signed-off-by: Andre McCurdy <armccurdy@gmail.com>
>> ---
>>  meta/lib/oe/rootfs.py | 9 ++++++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/meta/lib/oe/rootfs.py b/meta/lib/oe/rootfs.py
>> index a348b97..32cce64 100644
>> --- a/meta/lib/oe/rootfs.py
>> +++ b/meta/lib/oe/rootfs.py
>> @@ -352,8 +352,8 @@ class Rootfs(object, metaclass=ABCMeta):
>>      """
>>      Create devfs:
>>      * IMAGE_DEVICE_TABLE is the old name to an absolute path to a device table file
>> -    * IMAGE_DEVICE_TABLES is a new name for a file, or list of files, seached
>> -      for in the BBPATH
>> +    * IMAGE_DEVICE_TABLES is the new name for a file, or list of files, which may be
>> +      either absolute paths or paths relative to BBPATH.
>>      If neither are specified then the default name of files/device_table-minimal.txt
>>      is searched for in the BBPATH (same as the old version.)
>>      """
>> @@ -367,7 +367,10 @@ class Rootfs(object, metaclass=ABCMeta):
>>              if devtables is None:
>>                  devtables = 'files/device_table-minimal.txt'
>>              for devtable in devtables.split():
>> -                devtable_list.append("%s" % bb.utils.which(self.d.getVar('BBPATH', True), devtable))
>> +                if os.path.isabs(devtable):
>> +                    devtable_list.append(devtable)
>> +                else:
>> +                    devtable_list.append("%s" % bb.utils.which(self.d.getVar('BBPATH', True), devtable))
>
> I'm wondering if maybe this is a bug in "bb.utils.which", I thought it would
> return back -- in the case of an absolute path -- if the thing existed or not,
> otherwise would search the BBPATH.  (I'm using the command line version of
> 'which' as my behavioral thinking.)

Hmmm. The behaviour of "bb.utils.which" in the case of an absolute
path is actually to return back the absolute path - but only if the
file exists. If the file doesn't exist, you get back an empty string.
Unfortunately all the test cases I was trying were files which didn't
exist.

But perhaps the bigger issue is that result returned by
self._exec_shell_cmd() isn't checked, so it doesn't really matter
whether devtable_list ends up containing an empty string or an
absolute path to a file which doesn't exist. There's no error or
feedback to the user that something has gone wrong in either case.

Is it really intentional to silently skip over IMAGE_DEVICE_TABLES
entries which don't exist?

> If it is an issue, then it would need a fix in bitbake, and I believe the
> existing code should then work.
>
> (Asking about the behavior on the bitbake list might be worth while in this case.)
>
>>
>>          for devtable in devtable_list:
>>              self._exec_shell_cmd(["makedevs", "-r",
>>
>

Re: [PATCH] rootfs.py: support absolute paths in IMAGE_DEVICE_TABLES

[Mark Hatle]

On 9/12/16 10:20 PM, Andre McCurdy wrote:
> On Mon, Sep 12, 2016 at 5:17 PM, Mark Hatle <mark.hatle@windriver.com> wrote:
>> On 9/12/16 6:00 PM, Andre McCurdy wrote:
>>> Paths relative to BBPATH are fine if device table files are always
>>> static and contained somewhere within a meta layer. However if device
>>> tables are created dynamically, they will be located somewhere within
>>> ${TMPDIR} and most conveniently referenced via an absolute path.
>>>
>>> The legacy IMAGE_DEVICE_TABLE variable supported an absolute path,
>>> therefore make IMAGE_DEVICE_TABLES support absolute paths too, to
>>> avoid users who need dynamic device table files being dependent on
>>> the legacy variable.
>>>
>>> Signed-off-by: Andre McCurdy <armccurdy@gmail.com>
>>> ---
>>>  meta/lib/oe/rootfs.py | 9 ++++++---
>>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/meta/lib/oe/rootfs.py b/meta/lib/oe/rootfs.py
>>> index a348b97..32cce64 100644
>>> --- a/meta/lib/oe/rootfs.py
>>> +++ b/meta/lib/oe/rootfs.py
>>> @@ -352,8 +352,8 @@ class Rootfs(object, metaclass=ABCMeta):
>>>      """
>>>      Create devfs:
>>>      * IMAGE_DEVICE_TABLE is the old name to an absolute path to a device table file
>>> -    * IMAGE_DEVICE_TABLES is a new name for a file, or list of files, seached
>>> -      for in the BBPATH
>>> +    * IMAGE_DEVICE_TABLES is the new name for a file, or list of files, which may be
>>> +      either absolute paths or paths relative to BBPATH.
>>>      If neither are specified then the default name of files/device_table-minimal.txt
>>>      is searched for in the BBPATH (same as the old version.)
>>>      """
>>> @@ -367,7 +367,10 @@ class Rootfs(object, metaclass=ABCMeta):
>>>              if devtables is None:
>>>                  devtables = 'files/device_table-minimal.txt'
>>>              for devtable in devtables.split():
>>> -                devtable_list.append("%s" % bb.utils.which(self.d.getVar('BBPATH', True), devtable))
>>> +                if os.path.isabs(devtable):
>>> +                    devtable_list.append(devtable)
>>> +                else:
>>> +                    devtable_list.append("%s" % bb.utils.which(self.d.getVar('BBPATH', True), devtable))
>>
>> I'm wondering if maybe this is a bug in "bb.utils.which", I thought it would
>> return back -- in the case of an absolute path -- if the thing existed or not,
>> otherwise would search the BBPATH.  (I'm using the command line version of
>> 'which' as my behavioral thinking.)
> 
> Hmmm. The behaviour of "bb.utils.which" in the case of an absolute
> path is actually to return back the absolute path - but only if the
> file exists. If the file doesn't exist, you get back an empty string.
> Unfortunately all the test cases I was trying were files which didn't
> exist.
> 
> But perhaps the bigger issue is that result returned by
> self._exec_shell_cmd() isn't checked, so it doesn't really matter
> whether devtable_list ends up containing an empty string or an
> absolute path to a file which doesn't exist. There's no error or
> feedback to the user that something has gone wrong in either case.
> 
> Is it really intentional to silently skip over IMAGE_DEVICE_TABLES
> entries which don't exist?

Yes, that was the original implementation.  If the table wasn't found it was
just ignored.  That allows a distro to set a variety of names that a user can
then provide (or not).

The original implementation was focused on using bbpath and layers to add chunks
and provide layer specific configuration files, but it was also expected that in
the local.conf the user could provide their own, with an absolute path if necessary.

But like 'include', if it didn't exist it was just ignored.

--Mark

>> If it is an issue, then it would need a fix in bitbake, and I believe the
>> existing code should then work.
>>
>> (Asking about the behavior on the bitbake list might be worth while in this case.)
>>
>>>
>>>          for devtable in devtable_list:
>>>              self._exec_shell_cmd(["makedevs", "-r",
>>>
>>