[OE-core] [PATCH 5/5] rootfs.py: tweak _multilib_sanity_test for ipk incremental image generation

[Hongxu Jia]

I will rename _file_duplicate with _file_equal, and do
the necessary ajustment to avoid confusions.

Thank you for pointing out
V2 incoming

//Hongxu

On 02/18/2014 11:36 PM, Laurentiu Palcu wrote:
> On Tue, Feb 18, 2014 at 05:42:28PM +0800, Hongxu Jia wrote:
>> The _multilib_sanity_test installs multilib packages in a temporary
>> root fs, and compare with the current image to figure out duplicated
>> file that comes from different packages.
>>
>> While incremental image generation enabled and the previous image
>> existed, there was a Multilib check error:
>> ...
>> ERROR: Multilib check error: duplicate files tmp/work/qemux86_64-poky-
>> linux/core-image-minimal/1.0-r0/multilib/lib32/lib/libc.so.6 tmp/work/
>> qemux86_64-poky-linux/core-image-minimal/1.0-r0/rootfs/lib/libc.so.6
>> is not the same
>> ...
>>
>> The reason is the file in current image has been prelinked in previous
>> image generation and the file in a temporary root fs is not prelinked,
>> even though the files come from the same package, the Multilib check
>> considers they are different.
>>
>> [YOCTO #1894]
>> Signed-off-by: Hongxu Jia <hongxu.jia at windriver.com>
>> ---
>>   meta/lib/oe/rootfs.py | 56 +++++++++++++++++++++++++++++++++++++++++++++++----
>>   1 file changed, 52 insertions(+), 4 deletions(-)
>>
>> diff --git a/meta/lib/oe/rootfs.py b/meta/lib/oe/rootfs.py
>> index 3d7adf9..5054d1e 100644
>> --- a/meta/lib/oe/rootfs.py
>> +++ b/meta/lib/oe/rootfs.py
>> @@ -3,6 +3,8 @@ from oe.utils import execute_pre_post_process
>>   from oe.utils import contains as base_contains
>>   from oe.package_manager import *
>>   from oe.manifest import *
>> +import oe.path
>> +import filecmp
>>   import shutil
>>   import os
>>   import subprocess
>> @@ -458,13 +460,61 @@ class OpkgRootfs(Rootfs):
>>   
>>           bb.utils.remove(self.d.getVar('MULTILIB_TEMP_ROOTFS', True), True)
>>   
>> +    def _prelink_file(self, root_dir, filename):
>> +        bb.note('prelink %s in %s' % (filename, root_dir))
>> +        prelink_cfg = oe.path.join(root_dir,
>> +                                   self.d.expand('${sysconfdir}/prelink.conf'))
>> +        if not os.path.exists(prelink_cfg):
>> +            shutil.copy(self.d.expand('${STAGING_DIR_NATIVE}${sysconfdir_native}/prelink.conf'),
>> +                        prelink_cfg)
>> +
>> +        cmd_prelink = self.d.expand('${STAGING_DIR_NATIVE}${sbindir_native}/prelink')
>> +        self._exec_shell_cmd([cmd_prelink,
>> +                              '--root',
>> +                              root_dir,
>> +                              '-amR',
>> +                              '-N',
>> +                              '-c',
>> +                              self.d.expand('${sysconfdir}/prelink.conf')])
>> +
>> +    '''
>> +    Compare two files with the same key twice to see if they came
>> +    from the same package. If they are not same, they are duplicated
>> +    and come from different packages.
> I'm kind of confused by this comment. Doesn't same = duplicate? There
> might be a small confusion of terms here because the function's behavior
> is not as the name implies.
>
>> +    1st: Comapre them directly;
>> +    2nd: While incremental image creation is enabled, one of the
>> +         files could be probaly prelinked in the previous image
>> +         creation and the file has been changed, so we need to
>> +         prelink the other one and compare them.
>> +    '''
>> +    def _file_duplicate(self, key, f1, f2):
> Shouldn't be better to rename this function to something else in order
> to avoid confusion? Let's say: _files_are_equal() ?
>
>> +
>> +        if not os.path.exists(f1) or not os.path.exists(f2):
>> +            return False
>> +
>> +        # f1 is the same with f2, both of them were not prelinked
>> +        if filecmp.cmp(f1, f2):
>> +            return False
> filecmp.cmp() returns True if files are equal. Hence the confusion:
> _file_duplicate() returns False here, if files are equal... I think the logic
> is a little bit the other way around! :)
>
>> +
>> +        if self.image_rootfs not in f1:
>> +            self._prelink_file(f1.replace(key, ''), f1)
>> +
>> +        if self.image_rootfs not in f2:
>> +            self._prelink_file(f2.replace(key, ''), f2)
>> +
>> +        # f1 is the same with f2, both of them were prelinked
>> +        if filecmp.cmp(f1, f2):
>> +            return False
>> +
>> +        # Duplicated
>> +        return True
> here the return value matches the comment and the function name! :)
>> +
>>       """
>>       This function was reused from the old implementation.
>>       See commit: "image.bbclass: Added variables for multilib support." by
>>       Lianhao Lu.
>>       """
>>       def _multilib_sanity_test(self, dirs):
>> -        import filecmp
>>   
>>           allow_replace = self.d.getVar("MULTILIBRE_ALLOW_REP", True)
>>           if allow_replace is None:
>> @@ -486,9 +536,7 @@ class OpkgRootfs(Rootfs):
>>                           if allow_rep.match(key):
>>                               valid = True
>>                           else:
>> -                            if os.path.exists(files[key]) and \
>> -                               os.path.exists(item) and \
>> -                               not filecmp.cmp(files[key], item):
>> +                            if self._file_duplicate(key, files[key], item):
>>                                   valid = False
>>                                   bb.fatal("%s duplicate files %s %s is not the same\n" %
>>                                            (error_prompt, item, files[key]))
>> -- 
>> 1.8.1.2
>>