rootfs.py: support absolute paths in IMAGE_DEVICE_TABLES

PATCH In review Andre McCurdy 2 participants 4 messages Sep 12, 2016 – Sep 13, 2016

Share & tools

Andre McCurdy

author

2016-09-12 23:01 UTC #0 lore raw

PATCH 1/1 rootfs.py: support absolute paths in IMAGE_DEVICE_TABLES

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

---
 meta/lib/oe/rootfs.py | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

meta/lib/oe/rootfs.py +6 -3

index a348b97..32cce64 100644
@@ -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",

Signature · 1 line

1.9.1

Mark Hatle

2016-09-13 00:17 UTC #1 lore raw

Quoted 31 lines · 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 <[email protected]>
---
 meta/lib/oe/rootfs.py | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
meta/lib/oe/rootfs.py +2 -2
index a348b97..32cce64 100644
@@ -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.)
     """

meta/lib/oe/rootfs.py class Rootfs(object, metaclass=ABCMeta): · line 373

Quoted 8 lines

meta/lib/oe/rootfs.py +4 -1

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

Quoted 4 lines

meta/lib/oe/rootfs.py +0 -0

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

Andre McCurdy

author

2016-09-13 03:20 UTC #2 lore raw

Quoted 37 lines · Mark Hatle

Quoted 36 lines · 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 <[email protected]>
---
 meta/lib/oe/rootfs.py | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
meta/lib/oe/rootfs.py +3 -3
index a348b97..32cce64 100644
@@ -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):

Quoted 8 lines

meta/lib/oe/rootfs.py class Rootfs(object, metaclass=ABCMeta): · line 373
Quoted 3 lines
meta/lib/oe/rootfs.py +3 -0
+                    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?

Quoted 10 lines

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.)
Quoted 4 lines
meta/lib/oe/rootfs.py +0 -0
         for devtable in devtable_list:
             self._exec_shell_cmd(["makedevs", "-r",
 

Mark Hatle

2016-09-13 14:02 UTC #3 lore raw

Quoted 53 lines · Andre McCurdy

Quoted 37 lines · Mark Hatle
Quoted 36 lines · 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 <[email protected]>
---
 meta/lib/oe/rootfs.py | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
meta/lib/oe/rootfs.py +3 -3
index a348b97..32cce64 100644
@@ -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):
Quoted 8 lines
meta/lib/oe/rootfs.py class Rootfs(object, metaclass=ABCMeta): · line 373
Quoted 3 lines
meta/lib/oe/rootfs.py +3 -0
+                    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.

Quoted 8 lines

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

Quoted 10 lines

Quoted 10 lines
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.)
Quoted 4 lines
meta/lib/oe/rootfs.py +0 -0
         for devtable in devtable_list:
             self._exec_shell_cmd(["makedevs", "-r",
 