Re: [PATCH 1/1] linux-firmware: remove hard-coded paths

Ray, Ian (GE Healthcare)
2016-01-05T14:04:18Z

On 01/05/2016 03:26 AM, Matthias Schiffer wrote:
> On 01/05/2016 01:28 AM, Mark Hatle wrote:
>> On 1/4/16 5:57 PM, Matthias Schiffer wrote:
>>> On 01/04/2016 11:56 PM, Mark Hatle wrote:
>>>> On 1/4/16 4:26 PM, Matthias Schiffer wrote:
>>>>> On 01/04/2016 05:32 PM, Mark Hatle wrote:
>>>>>> On 1/4/16 10:11 AM, Matthias Schiffer wrote:
>>>>>>> On 01/04/2016 02:14 PM, Ian Ray wrote:
>>>>>>>> The recipe uses hard-coded paths (specifically /lib) in do_install
>>>>>>>> and in FILES, however on a merged /usr system this directory might
>>>>>>>> not exist. Prefer base_libdir.
>>>>>>>>
>>>>>>>> Signed-off-by: Ian Ray <ian.ray@ge.com>
>>>>>>>> ---
>>>>>>>
>>>>>>> This should use nonarch_base_libdir, base_libdir defaults to /lib64 on
>>>>>>> ppc64, which is not where the firmware is expected.
>>>>>>>
>>>>>>
>>>>>> At a minimum, I would agree nonarch_base_libdir, however..
>>>>>>
>>>>>> I believe that the kernel loader/modules/tools themselves actually have '/lib'
>>>>>> hard coded into them. This is the reason why /lib/firmware was used and not one
>>>>>> of the variables.
>>>>>>
>>>>>> This is one of the cases were /lib is actually correct, since that is what the
>>>>>> system is expecting. We can make some kind of accommodation for systems where
>>>>>> /lib -> /usr/lib... but that should be done inside of the filesystem setup
>>>>>> processing and not the package itself. (I'm referring to the
>>>>>> 'meta/files/fs-perms.txt' file.
>>>>>>
>>>>>> --Mark

Ah, that makes sense.

<snip>

>> You conditionalize it by providing different default fs-perms files -- OR since
>> more then one file can be loaded -- additional fs-perms with the permutations
>> you desire.
>>
>> I could easily see having an files/fs-perms-usr-only.txt and an
>> files/fs-perms-no-usr.txt
>>
>> Where the first would be something like:
>>
>> # Define symlinks from base directories to their prefix versions
>> /bin link ${bindir}
>> /sbin link ${sbindir}
>> ...

This is very interesting -- thank you.

I tried patching our additional fs-perms file as Mark suggested. This resulted in
a build failure during packaging of a recipe. The package directory looks odd:

ls -l /home/ian/myproduct/build/tmp/work/myproduct-linux-gnueabi/
myrecipe/4.3-r0/package
total 4
drwxr-xr-x. 2 ian ian 4096 Jan 5 11:30 boot
drwxr-xr-x. 4 ian ian 56 Jan 5 11:30 etc
lrwxrwxrwx. 1 ian ian 8 Jan 5 11:33 lib -> /usr/lib
drwxr-xr-x. 3 ian ian 16 Jan 5 11:33 usr

This can be fixed by teaching classes/package.bbclass to _not_ use the leading
slash when making the link. Not sure if that is unacceptably hacky? But it must
be better than leaking to host filesystem.

diff --git a/meta/classes/package.bbclass b/meta/classes/package.bbclass
index a86b680..59ef447 100644
--- a/meta/classes/package.bbclass
+++ b/meta/classes/package.bbclass
@@ -774,6 +774,7 @@ python fixup_perms () {
link = fs_perms_table[dir].link
if link[0] == "/":
target = dvar + link
+ link = link[1:]
ptarget = link
else:
target = os.path.join(os.path.dirname(origin), link)

Once past that problem there is a QA problem since do_package now installs
to /usr/lib (because of fs-perms) instead of /lib which is mentioned in FILES.

Joshua's patches for merged /usr touched FILES in _some_ cases (for example
73a6fe958f47642d18ba0098cfd45c3520d53560) but linux-firmware would require
considerably more effort. Is that an argument in favour of nonarch_base_libdir
instead of literal "/lib" or is there some other technique to address this?

Ian