Patchwork [5/5] rootfs.py: tweak _multilib_sanity_test for ipk incremental image generation

login
register
mail settings
Submitter Hongxu Jia
Date Feb. 18, 2014, 9:42 a.m.
Message ID <8581ec40c107111a8001742d8b636a76910f5f5f.1392716065.git.hongxu.jia@windriver.com>
Download mbox | patch
Permalink /patch/66905/
State New
Headers show

Comments

Hongxu Jia - Feb. 18, 2014, 9:42 a.m.
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@windriver.com>
---
 meta/lib/oe/rootfs.py | 56 +++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 52 insertions(+), 4 deletions(-)
Laurentiu Palcu - Feb. 18, 2014, 3:36 p.m.
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@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
>
Hongxu Jia - Feb. 19, 2014, 9:51 a.m.
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@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
>>

Patch

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.
+    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):
+
+        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
+
+        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
+
     """
     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]))