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

Mark Hatle
2016-01-05T14:32:46Z

On 1/5/16 8:04 AM, Ray, Ian (GE Healthcare) wrote:
> 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

We should look into this further. Pseudo (the fakeroot wrapper) should be
changing '/usr/lib' [in the cross build filesystem] to a relative path... It
will only be '/usr/lib' in the target version if we're in the pseudo environment
or the translation is off. (Maybe I'm remembering incorrectly, but that was my
memory.)

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

I'm definitely interested in understanding what may or may not be working
properly here.

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

What was the QA issue? About the absolute path name or something else?

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

For linux-firmware specifically.. we need to make sure the kernel and modules
(which use the firmware) use the same filesystem notion as what the
linux-firmware package is doing. In the past when I've looked, pretty much
everything was just hardcoded as '/lib/firmware'. So using that in the
packaging was correct for all three systems. (I don't believe patching the
three to use bitbake variables is necessarily a good idea -- since I'm sure
there are more instances of this type of usage that I'm unaware of.)

This is where using the fs-perms mechanism to move the files and setup the
symlink is the right approach in my opinion. The key thing is figuring out the
permutations of the libdir, nonarch libdir (since they may or may not be the
same) and dealing with any QA errors that may occur.

> Ian
>