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

Matthias Schiffer
2016-01-05T01:26:14Z

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

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
>>>>>
>>>>
>>>> There seem to be some intresting ideas going around about what can or
>>>> should be done via fs-perms.txt... AFAICT, fs-perms.txt can't move
>>>> around files, so moving files form /lib to /usr/lib must be done in the
>>>> package recipes themselves. (In my opinion, fs-perms.txt is a bad hack
>>>> for broken recipes that shouldn't exist anyways, but that's another
>>>> discussion)
>>>
>>> Since I wrote fs-perms.txt, I'll explain the purpose. Individual packages don't
>>> know if something is a directory, symbolic link, or what owner/group/permissions
>>> a system level directory should be set to.
>>>
>>> The entire purpose of it is to declare a common set of -system- directories.
>>> (Packages/layers can amend and override this as necessary to add their own
>>> system directories.)
>>>
>>> FYI System directories are things like /usr/bin. Having every package in the
>>> system need to define /usr/bin as a directory with an owner/group of root:root
>>> and permission of 0755 is a REALLY bad practice.. but putting this knowledge
>>> into a single file that synchronizes everything is very practical.
>>>
>>> When the system level directories are mapped to symlinks.. the case where
>>> everyone is trying to folks /usr -> / or /bin -> /usr/bin.. then it can
>>> AUTOMATICALLY map and move the files in these places..
>>
>> Thanks for the explanation, I asked a similar question a few weeks ago
>> and didn't get a satisfactory answer about what fs-perms can do. I'll
>> rethink my approach for the merged-usr patchset.
>
> (I've been out since near the beginning of December, but I'm back now..)
>
>> So the remaining issue is how to conditionalize this. I'd like to find a
>> solution which doesn't require distros to define their own fs-perms when
>> they just want to use a merged /usr dir.
>
> 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}
> ...
>
> fs-perms-no-user would be:
>
> # Define that $prefix as simply pointing back to root for compatibility
> /usr link /
>
>
> Then in your distro.conf file:
>
> FILESYSTEM_PERMS_TABLES = "files/fs-perms.txt"
> FILESYSTEM_PERMS_TABLES_append_usronly = " files/fs-perms-usr-only.txt"
> FILESYSTEM_PERMS_TABLES_append_nousr = " files/fs-perms-no-user.txt"
>
>
> The above will cause packages to produce the symlinks ONLY if the package itself
> creates a matching directory. If the package is moving all of it's files to the
> new configured location (from the distro.conf file) then no link is present.
>
> Assuming you want the links, I would add some kind of a change or configuration
> to the base-files to do this.
>
> (I did argue in the past, unsuccessfully, that base-files or something it calls
> should process the fs-perms files and simply generate a base configuration based
> on this setup. Ensuring symlinks and final directories were always created..
> but that was rejected at the time..)

I see. Do you think it would make sense to provide a fs-perms snippet in
oe-core for the Fedora-style merged-usr setup?

>
>>>
>>>> I think if a distro config changes any of the base paths
>>>> ({nonarch_,}base_libdir, base_{,s}bindir), *all* packages should respect
>>>> this. It's the distro's reponsiblity to create symlinks so everything is
>>>> found again at the expected paths (other examples for such hardcoded
>>>> paths: /bin/sh; the dynamic linker). See also my patchset I submitted to
>>>> this mailing list, which introduces a distro feature to have such
>>>> symlinks created by base-files.
>>>
>>> When this was written it was heavily argued against this knowledge being in
>>> base-files or base-dirs (suggested at the time) packages.
>>
>> Is that discussion archived somewhere? I'm interested in the
>> argumentation. Do any non-OE distros have a similar feature?
>
> This would have been discussed back in the original oe-core days. Likely around
> 2010 on the oe-core list....

Okay, I've found at least some fragments of the discussion.

>
>>>
>>> Defining a base setup, and then using a synchronization pass using the
>>> fs-perms.txt was the way to go.
>>>
>>> Note, fs-perms process is absolutely supposed to move files around -if- a
>>> symlink is generated.. i.e.:
>>>
>>> /lib -> /usr/lib
>>>
>>> if you write to /lib/firmware, the code is supposed to see the directory of
>>> '/lib', create a new /usr/lib (set perms properly) and move the contents if /lib
>>> to /usr/lib, then replace the directory with a symbolic link.
>>>
>>> If it's NOT doing that, lets fix it.
>>
>> I didn't try yet as I didn't now that it is supposed to to that.
>
> This is a good example where the upstream software is always going to use
> '/lib', but you want it to go somewhere else. Without this fs-perms mechanism,
> you would need to patch this package and potentially others. But by using
> fs-perms, they can continue to rely on their special knowledge of '/lib', and
> the system will fix it up before packaging to where the distribution actually
> wants the files.
>

I guess I'm fine with fs-perms.txt fixing permissions and owners, but
moving around files goes a bit too far in my opinion. Doing so will
potentially break relative symlinks and other relative paths used by
packages. I'd have implemented the "link" feature as a QA-only thing:
make the build fail if there are any files where a symlink is supposed
to go, but don't try to fix it up.

Another (more easily fixable) issue is that the automagic renaming
doesn't work if the target dir already exists (if I understand the code
correctly). So fixing up a package containing both files in /bin and
/usr/bin, where /bin is supposed to link to /usr/bin, will fail.

Matthias

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]