Openembedded Core Discussions
 help / color / mirror / Atom feed
* [PATCH] populate SDK: prepare calling of bb.utils for exceptions
@ 2015-10-15 17:27 Benjamin Esquivel
  2015-10-16  2:15 ` Christopher Larson
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Benjamin Esquivel @ 2015-10-15 17:27 UTC (permalink / raw)
  To: openembedded-core; +Cc: paul.eggleton

bb.utils.remove, bb.utils.movefile and bb.utils.mkdirhier can throw
exceptions that need handling and proper error messages.

[YOCTO#8213]

Signed-off-by: Benjamin Esquivel <benjamin.esquivel@linux.intel.com>
---
 meta/lib/oe/sdk.py | 81 ++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 57 insertions(+), 24 deletions(-)

diff --git a/meta/lib/oe/sdk.py b/meta/lib/oe/sdk.py
index 53da0f0..cefe3d8 100644
--- a/meta/lib/oe/sdk.py
+++ b/meta/lib/oe/sdk.py
@@ -25,7 +25,7 @@ class Sdk(object):
         else:
             self.manifest_dir = manifest_dir
 
-        bb.utils.remove(self.sdk_output, True)
+        self.remove(self.sdk_output, True)
 
         self.install_order = Manifest.INSTALL_ORDER
 
@@ -34,29 +34,60 @@ class Sdk(object):
         pass
 
     def populate(self):
-        bb.utils.mkdirhier(self.sdk_output)
+        self.mkdirhier(self.sdk_output)
+
 
         # call backend dependent implementation
         self._populate()
 
         # Don't ship any libGL in the SDK
-        bb.utils.remove(os.path.join(self.sdk_output, self.sdk_native_path,
-                                     self.d.getVar('libdir_nativesdk', True).strip('/'),
-                                     "libGL*"))
+        self.remove(os.path.join(self.sdk_output, self.sdk_native_path,
+                         self.d.getVar('libdir_nativesdk', True).strip('/'),
+                         "libGL*"))
 
         # Fix or remove broken .la files
-        bb.utils.remove(os.path.join(self.sdk_output, self.sdk_native_path,
-                                     self.d.getVar('libdir_nativesdk', True).strip('/'),
-                                     "*.la"))
+        self.remove(os.path.join(self.sdk_output, self.sdk_native_path,
+                         self.d.getVar('libdir_nativesdk', True).strip('/'),
+                         "*.la"))
 
         # Link the ld.so.cache file into the hosts filesystem
         link_name = os.path.join(self.sdk_output, self.sdk_native_path,
                                  self.sysconfdir, "ld.so.cache")
-        bb.utils.mkdirhier(os.path.dirname(link_name))
+        newdir = os.path.dirname(link_name)
+        self.mkdirhier(newdir)
+
         os.symlink("/etc/ld.so.cache", link_name)
 
         execute_pre_post_process(self.d, self.d.getVar('SDK_POSTPROCESS_COMMAND', True))
 
+    def movefile(self, sourcefile, destdir):
+        try:
+            # FIXME: this check of movefile's return code to None should be
+            # fixed within the function to use only exceptions to signal when
+            # something goes wrong
+            if (bb.utils.movefile(sourcefile, destdir) == None):
+                raise Exception("moving {} to {} failed".format(
+                                        sourcefile, destdir))
+        except Exception as e:
+            bb.warn(str(e))
+            bb.error("unable to place {} in final SDK location".format(
+                        sourcefile))
+
+    def mkdirhier(self, dirpath):
+        try:
+            bb.utils.mkdirhier(dirpath)
+        except OSError as e:
+            bb.warn(str(e))
+            bb.error("cannot make dir for SDK: {}".format(dirpath))
+
+    def remove(self, path, recurse=False):
+        try:
+            bb.utils.remove(path, recurse)
+        except Exception as e:
+            bb.warn(str(e))
+            bb.warn("cannot remove SDK dir: {}".format(path))
+
+
 
 class RpmSdk(Sdk):
     def __init__(self, d, manifest_dir=None):
@@ -143,15 +174,15 @@ class RpmSdk(Sdk):
                                             "lib",
                                             "rpm"
                                             )
-        bb.utils.mkdirhier(native_rpm_state_dir)
+        self.mkdirhier(native_rpm_state_dir)
         for f in glob.glob(os.path.join(self.sdk_output,
                                         "var",
                                         "lib",
                                         "rpm",
                                         "*")):
-            bb.utils.movefile(f, native_rpm_state_dir)
+            self.movefile(f, native_rpm_state_dir)
 
-        bb.utils.remove(os.path.join(self.sdk_output, "var"), True)
+        self.remove(os.path.join(self.sdk_output, "var"), True)
 
         # Move host sysconfig data
         native_sysconf_dir = os.path.join(self.sdk_output,
@@ -159,10 +190,10 @@ class RpmSdk(Sdk):
                                           self.d.getVar('sysconfdir',
                                                         True).strip('/'),
                                           )
-        bb.utils.mkdirhier(native_sysconf_dir)
+        self.mkdirhier(native_sysconf_dir)
         for f in glob.glob(os.path.join(self.sdk_output, "etc", "*")):
-            bb.utils.movefile(f, native_sysconf_dir)
-        bb.utils.remove(os.path.join(self.sdk_output, "etc"), True)
+            self.movefile(f, native_sysconf_dir)
+        self.remove(os.path.join(self.sdk_output, "etc"), True)
 
 
 class OpkgSdk(Sdk):
@@ -219,12 +250,12 @@ class OpkgSdk(Sdk):
         target_sysconfdir = os.path.join(self.sdk_target_sysroot, self.sysconfdir)
         host_sysconfdir = os.path.join(self.sdk_host_sysroot, self.sysconfdir)
 
-        bb.utils.mkdirhier(target_sysconfdir)
+        self.mkdirhier(target_sysconfdir)
         shutil.copy(self.target_conf, target_sysconfdir)
         os.chmod(os.path.join(target_sysconfdir,
                               os.path.basename(self.target_conf)), 0644)
 
-        bb.utils.mkdirhier(host_sysconfdir)
+        self.mkdirhier(host_sysconfdir)
         shutil.copy(self.host_conf, host_sysconfdir)
         os.chmod(os.path.join(host_sysconfdir,
                               os.path.basename(self.host_conf)), 0644)
@@ -232,11 +263,11 @@ class OpkgSdk(Sdk):
         native_opkg_state_dir = os.path.join(self.sdk_output, self.sdk_native_path,
                                              self.d.getVar('localstatedir_nativesdk', True).strip('/'),
                                              "lib", "opkg")
-        bb.utils.mkdirhier(native_opkg_state_dir)
+        self.mkdirhier(native_opkg_state_dir)
         for f in glob.glob(os.path.join(self.sdk_output, "var", "lib", "opkg", "*")):
-            bb.utils.movefile(f, native_opkg_state_dir)
+            self.movefile(f, native_opkg_state_dir)
 
-        bb.utils.remove(os.path.join(self.sdk_output, "var"), True)
+        self.remove(os.path.join(self.sdk_output, "var"), True)
 
 
 class DpkgSdk(Sdk):
@@ -264,7 +295,7 @@ class DpkgSdk(Sdk):
     def _copy_apt_dir_to(self, dst_dir):
         staging_etcdir_native = self.d.getVar("STAGING_ETCDIR_NATIVE", True)
 
-        bb.utils.remove(dst_dir, True)
+        self.remove(dst_dir, True)
 
         shutil.copytree(os.path.join(staging_etcdir_native, "apt"), dst_dir)
 
@@ -306,11 +337,13 @@ class DpkgSdk(Sdk):
 
         native_dpkg_state_dir = os.path.join(self.sdk_output, self.sdk_native_path,
                                              "var", "lib", "dpkg")
-        bb.utils.mkdirhier(native_dpkg_state_dir)
+        self.mkdirhier(native_dpkg_state_dir)
+
         for f in glob.glob(os.path.join(self.sdk_output, "var", "lib", "dpkg", "*")):
-            bb.utils.movefile(f, native_dpkg_state_dir)
+            self.movefile(f, native_dpkg_state_dir)
+
+        self.remove(os.path.join(self.sdk_output, "var"), True)
 
-        bb.utils.remove(os.path.join(self.sdk_output, "var"), True)
 
 
 def sdk_list_installed_packages(d, target, format=None, rootfs_dir=None):
-- 
2.4.3



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] populate SDK: prepare calling of bb.utils for exceptions
  2015-10-15 17:27 [PATCH] populate SDK: prepare calling of bb.utils for exceptions Benjamin Esquivel
@ 2015-10-16  2:15 ` Christopher Larson
  2015-10-16 20:28   ` Benjamin Esquivel
  2015-10-16 21:50   ` [PATCH V2] " Benjamin Esquivel
  2015-10-16  7:00 ` [PATCH] " Mike Looijmans
  2015-10-16 10:01 ` Richard Purdie
  2 siblings, 2 replies; 10+ messages in thread
From: Christopher Larson @ 2015-10-16  2:15 UTC (permalink / raw)
  To: Benjamin Esquivel
  Cc: paul.eggleton@linux.intel.com,
	Patches and discussions about the oe-core layer

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

On Thu, Oct 15, 2015 at 10:27 AM, Benjamin Esquivel <
benjamin.esquivel@linux.intel.com> wrote:

> +    def movefile(self, sourcefile, destdir):
> +        try:
> +            # FIXME: this check of movefile's return code to None should
> be
> +            # fixed within the function to use only exceptions to signal
> when
> +            # something goes wrong
> +            if (bb.utils.movefile(sourcefile, destdir) == None):
> +                raise Exception("moving {} to {} failed".format(
> +                                        sourcefile, destdir))
> +        except Exception as e:
> +            bb.warn(str(e))
> +            bb.error("unable to place {} in final SDK location".format(
> +                        sourcefile))
> +
> +    def mkdirhier(self, dirpath):
> +        try:
> +            bb.utils.mkdirhier(dirpath)
> +        except OSError as e:
> +            bb.warn(str(e))
> +            bb.error("cannot make dir for SDK: {}".format(dirpath))
>

Shouldn't this be fatal?
-- 
Christopher Larson
clarson at kergoth dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Maintainer - Tslib
Senior Software Engineer, Mentor Graphics

[-- Attachment #2: Type: text/html, Size: 1777 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] populate SDK: prepare calling of bb.utils for exceptions
  2015-10-15 17:27 [PATCH] populate SDK: prepare calling of bb.utils for exceptions Benjamin Esquivel
  2015-10-16  2:15 ` Christopher Larson
@ 2015-10-16  7:00 ` Mike Looijmans
  2015-10-16 20:27   ` Benjamin Esquivel
  2015-10-16 10:01 ` Richard Purdie
  2 siblings, 1 reply; 10+ messages in thread
From: Mike Looijmans @ 2015-10-16  7:00 UTC (permalink / raw)
  To: Benjamin Esquivel, openembedded-core; +Cc: paul.eggleton

On 15-10-15 19:27, Benjamin Esquivel wrote:
> bb.utils.remove, bb.utils.movefile and bb.utils.mkdirhier can throw
> exceptions that need handling and proper error messages.
>
> [YOCTO#8213]
...
> +    def mkdirhier(self, dirpath):
> +        try:
> +            bb.utils.mkdirhier(dirpath)
> +        except OSError as e:
> +            bb.warn(str(e))
> +            bb.error("cannot make dir for SDK: {}".format(dirpath))
...


I see two bad things happening here:

- This will print a message, but continue processing as if nothing bad 
happened, wreaking havoc later on when the caller might expect the directory 
to actually exist and raise exceptions that are much harder to solve.

- It loses the information in the original exception, and since it doesn't 
throw a new exception, the user is now left in the dark as to where the 
problem occured, since there's no stack trace now.


For what I see in this code, it will actually make thing worse by hiding 
errors and obscuring information. You're not "handling" the exception here, 
you're almost "ignoring" them, which isn't quite the same.



Kind regards,

Mike Looijmans
System Expert

TOPIC Embedded Products
Eindhovenseweg 32-C, NL-5683 KH Best
Postbus 440, NL-5680 AK Best
Telefoon: +31 (0) 499 33 69 79
Telefax: +31 (0) 499 33 69 70
E-mail: mike.looijmans@topicproducts.com
Website: www.topicproducts.com

Please consider the environment before printing this e-mail







^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] populate SDK: prepare calling of bb.utils for exceptions
  2015-10-15 17:27 [PATCH] populate SDK: prepare calling of bb.utils for exceptions Benjamin Esquivel
  2015-10-16  2:15 ` Christopher Larson
  2015-10-16  7:00 ` [PATCH] " Mike Looijmans
@ 2015-10-16 10:01 ` Richard Purdie
  2015-10-16 20:18   ` Benjamin Esquivel
  2 siblings, 1 reply; 10+ messages in thread
From: Richard Purdie @ 2015-10-16 10:01 UTC (permalink / raw)
  To: Benjamin Esquivel; +Cc: paul.eggleton, openembedded-core

On Thu, 2015-10-15 at 12:27 -0500, Benjamin Esquivel wrote:
> +            if (bb.utils.movefile(sourcefile, destdir) == None):
> +                raise Exception("moving {} to {} failed".format(
> +                                        sourcefile, destdir))

Its a minor issue, however rightly or wrongly, the vast majority of the
code base uses "xxx %s %s" % (var, var2) instead of .format(). Is there
a reason we should be using the latter as for consistency, the former
would seem to make things more standardised?

Cheers,

Richard





^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] populate SDK: prepare calling of bb.utils for exceptions
  2015-10-16 10:01 ` Richard Purdie
@ 2015-10-16 20:18   ` Benjamin Esquivel
  0 siblings, 0 replies; 10+ messages in thread
From: Benjamin Esquivel @ 2015-10-16 20:18 UTC (permalink / raw)
  To: Richard Purdie; +Cc: paul.eggleton, openembedded-core

On Fri, 2015-10-16 at 11:01 +0100, Richard Purdie wrote:
> On Thu, 2015-10-15 at 12:27 -0500, Benjamin Esquivel wrote:
> > +            if (bb.utils.movefile(sourcefile, destdir) == None):
> > +                raise Exception("moving {} to {} failed".format(
> > +                                        sourcefile, destdir))
> 
> Its a minor issue, however rightly or wrongly, the vast majority of
> the
> code base uses "xxx %s %s" % (var, var2) instead of .format(). Is
> there
> a reason we should be using the latter as for consistency, the former
> would seem to make things more standardised?
> 
No other reason than .format being pythonic and % being an old way of
string formatting. I can move this to % instead on the grounds of
standardizing. 
> Cheers,
> 
> Richard
> 
> 
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] populate SDK: prepare calling of bb.utils for exceptions
  2015-10-16  7:00 ` [PATCH] " Mike Looijmans
@ 2015-10-16 20:27   ` Benjamin Esquivel
  0 siblings, 0 replies; 10+ messages in thread
From: Benjamin Esquivel @ 2015-10-16 20:27 UTC (permalink / raw)
  To: Mike Looijmans, openembedded-core; +Cc: paul.eggleton

Hello Mike, comments below:

On Fri, 2015-10-16 at 09:00 +0200, Mike Looijmans wrote:
> On 15-10-15 19:27, Benjamin Esquivel wrote:
> > bb.utils.remove, bb.utils.movefile and bb.utils.mkdirhier can throw
> > exceptions that need handling and proper error messages.
> > 
> > [YOCTO#8213]
> ...
> > +    def mkdirhier(self, dirpath):
> > +        try:
> > +            bb.utils.mkdirhier(dirpath)
> > +        except OSError as e:
> > +            bb.warn(str(e))
> > +            bb.error("cannot make dir for SDK:
> > {}".format(dirpath))
> ...
> 
> 
> I see two bad things happening here:
> 
> - This will print a message, but continue processing as if nothing
> bad 
> happened, wreaking havoc later on when the caller might expect the
> directory 
> to actually exist and raise exceptions that are much harder to solve.
The exception is not re-raised since this is the appropriate level of
handling and you made me realize that this specific case will only be
reached if the dir is not present and unable to be created so it should
be changed for a bb.fatal instead of bb.error. thanks.
> 
> - It loses the information in the original exception, and since it
> doesn't 
> throw a new exception, the user is now left in the dark as to where
> the 
> problem occured, since there's no stack trace now.
> 
I will throw the trace to the log
> 
> For what I see in this code, it will actually make thing worse by
> hiding 
> errors and obscuring information. You're not "handling" the exception
> here, 
> you're almost "ignoring" them, which isn't quite the same.
> 
not agreed, current code hides errors in return codes which are not
even checked.
> 
> 
> Kind regards,
> 
> Mike Looijmans
> System Expert
> 
> TOPIC Embedded Products
> Eindhovenseweg 32-C, NL-5683 KH Best
> Postbus 440, NL-5680 AK Best
> Telefoon: +31 (0) 499 33 69 79
> Telefax: +31 (0) 499 33 69 70
> E-mail: mike.looijmans@topicproducts.com
> Website: www.topicproducts.com
> 
> Please consider the environment before printing this e-mail
> 
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] populate SDK: prepare calling of bb.utils for exceptions
  2015-10-16  2:15 ` Christopher Larson
@ 2015-10-16 20:28   ` Benjamin Esquivel
  2015-10-16 21:50   ` [PATCH V2] " Benjamin Esquivel
  1 sibling, 0 replies; 10+ messages in thread
From: Benjamin Esquivel @ 2015-10-16 20:28 UTC (permalink / raw)
  To: Christopher Larson
  Cc: paul.eggleton@linux.intel.com,
	Patches and discussions about the oe-core layer

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

On Thu, 2015-10-15 at 19:15 -0700, Christopher Larson wrote:
> 
> On Thu, Oct 15, 2015 at 10:27 AM, Benjamin Esquivel <
> benjamin.esquivel@linux.intel.com> wrote:
> > +    def movefile(self, sourcefile, destdir):
> > +        try:
> > +            # FIXME: this check of movefile's return code to None
> > should be
> > +            # fixed within the function to use only exceptions to
> > signal when
> > +            # something goes wrong
> > +            if (bb.utils.movefile(sourcefile, destdir) == None):
> > +                raise Exception("moving {} to {} failed".format(
> > +                                        sourcefile, destdir))
> > +        except Exception as e:
> > +            bb.warn(str(e))
> > +            bb.error("unable to place {} in final SDK
> > location".format(
> > +                        sourcefile))
> > +
> > +    def mkdirhier(self, dirpath):
> > +        try:
> > +            bb.utils.mkdirhier(dirpath)
> > +        except OSError as e:
> > +            bb.warn(str(e))
> > +            bb.error("cannot make dir for SDK:
> > {}".format(dirpath))
> > 
> Shouldn't this be fatal?
correct, will change to fatal. Thanks.

[-- Attachment #2: Type: text/html, Size: 2111 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH V2] populate SDK: prepare calling of bb.utils for exceptions
  2015-10-16  2:15 ` Christopher Larson
  2015-10-16 20:28   ` Benjamin Esquivel
@ 2015-10-16 21:50   ` Benjamin Esquivel
  2015-10-19  5:27     ` Mike Looijmans
  1 sibling, 1 reply; 10+ messages in thread
From: Benjamin Esquivel @ 2015-10-16 21:50 UTC (permalink / raw)
  To: openembedded-core; +Cc: paul.eggleton, mike.looijmans

bb.utils.remove, bb.utils.movefile and bb.utils.mkdirhier can throw
exceptions that need handling and proper error messages
more work is required for these methods to handle properly the
exceptions that can be raised within the various OS calls they make
but this is a start to at least not hide the errors in the requested
operations

[YOCTO#8213]

Signed-off-by: Benjamin Esquivel <benjamin.esquivel@linux.intel.com>
---
 meta/lib/oe/sdk.py | 76 +++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 52 insertions(+), 24 deletions(-)

diff --git a/meta/lib/oe/sdk.py b/meta/lib/oe/sdk.py
index 53da0f0..3103f48 100644
--- a/meta/lib/oe/sdk.py
+++ b/meta/lib/oe/sdk.py
@@ -5,6 +5,7 @@ from oe.package_manager import *
 import os
 import shutil
 import glob
+import traceback
 
 
 class Sdk(object):
@@ -25,7 +26,7 @@ class Sdk(object):
         else:
             self.manifest_dir = manifest_dir
 
-        bb.utils.remove(self.sdk_output, True)
+        self.remove(self.sdk_output, True)
 
         self.install_order = Manifest.INSTALL_ORDER
 
@@ -34,29 +35,56 @@ class Sdk(object):
         pass
 
     def populate(self):
-        bb.utils.mkdirhier(self.sdk_output)
+        self.mkdirhier(self.sdk_output)
 
         # call backend dependent implementation
         self._populate()
 
         # Don't ship any libGL in the SDK
-        bb.utils.remove(os.path.join(self.sdk_output, self.sdk_native_path,
-                                     self.d.getVar('libdir_nativesdk', True).strip('/'),
-                                     "libGL*"))
+        self.remove(os.path.join(self.sdk_output, self.sdk_native_path,
+                         self.d.getVar('libdir_nativesdk', True).strip('/'),
+                         "libGL*"))
 
         # Fix or remove broken .la files
-        bb.utils.remove(os.path.join(self.sdk_output, self.sdk_native_path,
-                                     self.d.getVar('libdir_nativesdk', True).strip('/'),
-                                     "*.la"))
+        self.remove(os.path.join(self.sdk_output, self.sdk_native_path,
+                         self.d.getVar('libdir_nativesdk', True).strip('/'),
+                         "*.la"))
 
         # Link the ld.so.cache file into the hosts filesystem
         link_name = os.path.join(self.sdk_output, self.sdk_native_path,
                                  self.sysconfdir, "ld.so.cache")
-        bb.utils.mkdirhier(os.path.dirname(link_name))
+        self.mkdirhier(os.path.dirname(link_name))
         os.symlink("/etc/ld.so.cache", link_name)
 
         execute_pre_post_process(self.d, self.d.getVar('SDK_POSTPROCESS_COMMAND', True))
 
+    def movefile(self, sourcefile, destdir):
+        try:
+            # FIXME: this check of movefile's return code to None should be
+            # fixed within the function to use only exceptions to signal when
+            # something goes wrong
+            if (bb.utils.movefile(sourcefile, destdir) == None):
+                raise OSError("moving %s to %s failed"
+                        %(sourcefile, destdir))
+        #FIXME: using umbrella exc catching because bb.utils method raises it
+        except Exception as e:
+            bb.debug(1, "printing the stack trace\n %s" %traceback.format_exc())
+            bb.error("unable to place %s in final SDK location" % sourcefile)
+
+    def mkdirhier(self, dirpath):
+        try:
+            bb.utils.mkdirhier(dirpath)
+        except OSError as e:
+            bb.debug(1, "printing the stack trace\n %s" %traceback.format_exc())
+            bb.fatal("cannot make dir for SDK: %s" % dirpath)
+
+    def remove(self, path, recurse=False):
+        try:
+            bb.utils.remove(path, recurse)
+        #FIXME: using umbrella exc catching because bb.utils method raises it
+        except Exception as e:
+            bb.debug(1, "printing the stack trace\n %s" %traceback.format_exc())
+            bb.warn("cannot remove SDK dir: %s" % path)
 
 class RpmSdk(Sdk):
     def __init__(self, d, manifest_dir=None):
@@ -143,15 +171,15 @@ class RpmSdk(Sdk):
                                             "lib",
                                             "rpm"
                                             )
-        bb.utils.mkdirhier(native_rpm_state_dir)
+        self.mkdirhier(native_rpm_state_dir)
         for f in glob.glob(os.path.join(self.sdk_output,
                                         "var",
                                         "lib",
                                         "rpm",
                                         "*")):
-            bb.utils.movefile(f, native_rpm_state_dir)
+            self.movefile(f, native_rpm_state_dir)
 
-        bb.utils.remove(os.path.join(self.sdk_output, "var"), True)
+        self.remove(os.path.join(self.sdk_output, "var"), True)
 
         # Move host sysconfig data
         native_sysconf_dir = os.path.join(self.sdk_output,
@@ -159,10 +187,10 @@ class RpmSdk(Sdk):
                                           self.d.getVar('sysconfdir',
                                                         True).strip('/'),
                                           )
-        bb.utils.mkdirhier(native_sysconf_dir)
+        self.mkdirhier(native_sysconf_dir)
         for f in glob.glob(os.path.join(self.sdk_output, "etc", "*")):
-            bb.utils.movefile(f, native_sysconf_dir)
-        bb.utils.remove(os.path.join(self.sdk_output, "etc"), True)
+            self.movefile(f, native_sysconf_dir)
+        self.remove(os.path.join(self.sdk_output, "etc"), True)
 
 
 class OpkgSdk(Sdk):
@@ -219,12 +247,12 @@ class OpkgSdk(Sdk):
         target_sysconfdir = os.path.join(self.sdk_target_sysroot, self.sysconfdir)
         host_sysconfdir = os.path.join(self.sdk_host_sysroot, self.sysconfdir)
 
-        bb.utils.mkdirhier(target_sysconfdir)
+        self.mkdirhier(target_sysconfdir)
         shutil.copy(self.target_conf, target_sysconfdir)
         os.chmod(os.path.join(target_sysconfdir,
                               os.path.basename(self.target_conf)), 0644)
 
-        bb.utils.mkdirhier(host_sysconfdir)
+        self.mkdirhier(host_sysconfdir)
         shutil.copy(self.host_conf, host_sysconfdir)
         os.chmod(os.path.join(host_sysconfdir,
                               os.path.basename(self.host_conf)), 0644)
@@ -232,11 +260,11 @@ class OpkgSdk(Sdk):
         native_opkg_state_dir = os.path.join(self.sdk_output, self.sdk_native_path,
                                              self.d.getVar('localstatedir_nativesdk', True).strip('/'),
                                              "lib", "opkg")
-        bb.utils.mkdirhier(native_opkg_state_dir)
+        self.mkdirhier(native_opkg_state_dir)
         for f in glob.glob(os.path.join(self.sdk_output, "var", "lib", "opkg", "*")):
-            bb.utils.movefile(f, native_opkg_state_dir)
+            self.movefile(f, native_opkg_state_dir)
 
-        bb.utils.remove(os.path.join(self.sdk_output, "var"), True)
+        self.remove(os.path.join(self.sdk_output, "var"), True)
 
 
 class DpkgSdk(Sdk):
@@ -264,7 +292,7 @@ class DpkgSdk(Sdk):
     def _copy_apt_dir_to(self, dst_dir):
         staging_etcdir_native = self.d.getVar("STAGING_ETCDIR_NATIVE", True)
 
-        bb.utils.remove(dst_dir, True)
+        self.remove(dst_dir, True)
 
         shutil.copytree(os.path.join(staging_etcdir_native, "apt"), dst_dir)
 
@@ -306,11 +334,11 @@ class DpkgSdk(Sdk):
 
         native_dpkg_state_dir = os.path.join(self.sdk_output, self.sdk_native_path,
                                              "var", "lib", "dpkg")
-        bb.utils.mkdirhier(native_dpkg_state_dir)
+        self.mkdirhier(native_dpkg_state_dir)
         for f in glob.glob(os.path.join(self.sdk_output, "var", "lib", "dpkg", "*")):
-            bb.utils.movefile(f, native_dpkg_state_dir)
+            self.movefile(f, native_dpkg_state_dir)
+        self.remove(os.path.join(self.sdk_output, "var"), True)
 
-        bb.utils.remove(os.path.join(self.sdk_output, "var"), True)
 
 
 def sdk_list_installed_packages(d, target, format=None, rootfs_dir=None):
-- 
2.4.3



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH V2] populate SDK: prepare calling of bb.utils for exceptions
  2015-10-16 21:50   ` [PATCH V2] " Benjamin Esquivel
@ 2015-10-19  5:27     ` Mike Looijmans
  2015-10-19 15:28       ` [PATCH V3] " Benjamin Esquivel
  0 siblings, 1 reply; 10+ messages in thread
From: Mike Looijmans @ 2015-10-19  5:27 UTC (permalink / raw)
  To: Benjamin Esquivel, openembedded-core; +Cc: paul.eggleton

On 16-10-15 23:50, Benjamin Esquivel wrote:
> bb.utils.remove, bb.utils.movefile and bb.utils.mkdirhier can throw
> exceptions that need handling and proper error messages
> more work is required for these methods to handle properly the
> exceptions that can be raised within the various OS calls they make
> but this is a start to at least not hide the errors in the requested
> operations
>
> [YOCTO#8213]
>
> Signed-off-by: Benjamin Esquivel <benjamin.esquivel@linux.intel.com>
> ---
...
> +    def movefile(self, sourcefile, destdir):
> +        try:
> +            # FIXME: this check of movefile's return code to None should be
> +            # fixed within the function to use only exceptions to signal when
> +            # something goes wrong
> +            if (bb.utils.movefile(sourcefile, destdir) == None):
> +                raise OSError("moving %s to %s failed"
> +                        %(sourcefile, destdir))
> +        #FIXME: using umbrella exc catching because bb.utils method raises it
> +        except Exception as e:
> +            bb.debug(1, "printing the stack trace\n %s" %traceback.format_exc())
> +            bb.error("unable to place %s in final SDK location" % sourcefile)
> +
> +    def mkdirhier(self, dirpath):
> +        try:
> +            bb.utils.mkdirhier(dirpath)
> +        except OSError as e:
> +            bb.debug(1, "printing the stack trace\n %s" %traceback.format_exc())
> +            bb.fatal("cannot make dir for SDK: %s" % dirpath)
> +
> +    def remove(self, path, recurse=False):
> +        try:
> +            bb.utils.remove(path, recurse)
> +        #FIXME: using umbrella exc catching because bb.utils method raises it
> +        except Exception as e:
> +            bb.debug(1, "printing the stack trace\n %s" %traceback.format_exc())
> +            bb.warn("cannot remove SDK dir: %s" % path)


All these methods have a "self" in their argument list, but don't use it. Make 
them static functions (inside or outside the class, that's your call. I'd put 
them outside, they're totally unrelated).



Kind regards,

Mike Looijmans
System Expert

TOPIC Embedded Products
Eindhovenseweg 32-C, NL-5683 KH Best
Postbus 440, NL-5680 AK Best
Telefoon: +31 (0) 499 33 69 79
Telefax: +31 (0) 499 33 69 70
E-mail: mike.looijmans@topicproducts.com
Website: www.topicproducts.com

Please consider the environment before printing this e-mail

Visit us at : Aerospace Electrical Systems Expo Europe which will be held from 17.11.2015 till 19.11.2015, Findorffstra�e 101 Bremen, Germany, Hall 5, stand number C65
http://www.aesexpo.eu




^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH V3] populate SDK: prepare calling of bb.utils for exceptions
  2015-10-19  5:27     ` Mike Looijmans
@ 2015-10-19 15:28       ` Benjamin Esquivel
  0 siblings, 0 replies; 10+ messages in thread
From: Benjamin Esquivel @ 2015-10-19 15:28 UTC (permalink / raw)
  To: openembedded-core; +Cc: paul.eggleton, mike.looijmans

bb.utils.remove, bb.utils.movefile and bb.utils.mkdirhier can throw
exceptions that need handling and proper error messages
more work is required for these methods to handle properly the
exceptions that can be raised within the various OS calls they make
but this is a start to at least not hide the errors in the requested
operations

[YOCTO#8213]

Signed-off-by: Benjamin Esquivel <benjamin.esquivel@linux.intel.com>
---
 meta/lib/oe/sdk.py | 78 +++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 54 insertions(+), 24 deletions(-)

diff --git a/meta/lib/oe/sdk.py b/meta/lib/oe/sdk.py
index 53da0f0..4d3534a 100644
--- a/meta/lib/oe/sdk.py
+++ b/meta/lib/oe/sdk.py
@@ -5,6 +5,7 @@ from oe.package_manager import *
 import os
 import shutil
 import glob
+import traceback
 
 
 class Sdk(object):
@@ -25,7 +26,7 @@ class Sdk(object):
         else:
             self.manifest_dir = manifest_dir
 
-        bb.utils.remove(self.sdk_output, True)
+        remove(self.sdk_output, True)
 
         self.install_order = Manifest.INSTALL_ORDER
 
@@ -34,25 +35,25 @@ class Sdk(object):
         pass
 
     def populate(self):
-        bb.utils.mkdirhier(self.sdk_output)
+        mkdirhier(self.sdk_output)
 
         # call backend dependent implementation
         self._populate()
 
         # Don't ship any libGL in the SDK
-        bb.utils.remove(os.path.join(self.sdk_output, self.sdk_native_path,
-                                     self.d.getVar('libdir_nativesdk', True).strip('/'),
-                                     "libGL*"))
+        remove(os.path.join(self.sdk_output, self.sdk_native_path,
+                         self.d.getVar('libdir_nativesdk', True).strip('/'),
+                         "libGL*"))
 
         # Fix or remove broken .la files
-        bb.utils.remove(os.path.join(self.sdk_output, self.sdk_native_path,
-                                     self.d.getVar('libdir_nativesdk', True).strip('/'),
-                                     "*.la"))
+        remove(os.path.join(self.sdk_output, self.sdk_native_path,
+                         self.d.getVar('libdir_nativesdk', True).strip('/'),
+                         "*.la"))
 
         # Link the ld.so.cache file into the hosts filesystem
         link_name = os.path.join(self.sdk_output, self.sdk_native_path,
                                  self.sysconfdir, "ld.so.cache")
-        bb.utils.mkdirhier(os.path.dirname(link_name))
+        mkdirhier(os.path.dirname(link_name))
         os.symlink("/etc/ld.so.cache", link_name)
 
         execute_pre_post_process(self.d, self.d.getVar('SDK_POSTPROCESS_COMMAND', True))
@@ -143,15 +144,15 @@ class RpmSdk(Sdk):
                                             "lib",
                                             "rpm"
                                             )
-        bb.utils.mkdirhier(native_rpm_state_dir)
+        mkdirhier(native_rpm_state_dir)
         for f in glob.glob(os.path.join(self.sdk_output,
                                         "var",
                                         "lib",
                                         "rpm",
                                         "*")):
-            bb.utils.movefile(f, native_rpm_state_dir)
+            movefile(f, native_rpm_state_dir)
 
-        bb.utils.remove(os.path.join(self.sdk_output, "var"), True)
+        remove(os.path.join(self.sdk_output, "var"), True)
 
         # Move host sysconfig data
         native_sysconf_dir = os.path.join(self.sdk_output,
@@ -159,10 +160,10 @@ class RpmSdk(Sdk):
                                           self.d.getVar('sysconfdir',
                                                         True).strip('/'),
                                           )
-        bb.utils.mkdirhier(native_sysconf_dir)
+        mkdirhier(native_sysconf_dir)
         for f in glob.glob(os.path.join(self.sdk_output, "etc", "*")):
-            bb.utils.movefile(f, native_sysconf_dir)
-        bb.utils.remove(os.path.join(self.sdk_output, "etc"), True)
+            movefile(f, native_sysconf_dir)
+        remove(os.path.join(self.sdk_output, "etc"), True)
 
 
 class OpkgSdk(Sdk):
@@ -219,12 +220,12 @@ class OpkgSdk(Sdk):
         target_sysconfdir = os.path.join(self.sdk_target_sysroot, self.sysconfdir)
         host_sysconfdir = os.path.join(self.sdk_host_sysroot, self.sysconfdir)
 
-        bb.utils.mkdirhier(target_sysconfdir)
+        mkdirhier(target_sysconfdir)
         shutil.copy(self.target_conf, target_sysconfdir)
         os.chmod(os.path.join(target_sysconfdir,
                               os.path.basename(self.target_conf)), 0644)
 
-        bb.utils.mkdirhier(host_sysconfdir)
+        mkdirhier(host_sysconfdir)
         shutil.copy(self.host_conf, host_sysconfdir)
         os.chmod(os.path.join(host_sysconfdir,
                               os.path.basename(self.host_conf)), 0644)
@@ -232,11 +233,11 @@ class OpkgSdk(Sdk):
         native_opkg_state_dir = os.path.join(self.sdk_output, self.sdk_native_path,
                                              self.d.getVar('localstatedir_nativesdk', True).strip('/'),
                                              "lib", "opkg")
-        bb.utils.mkdirhier(native_opkg_state_dir)
+        mkdirhier(native_opkg_state_dir)
         for f in glob.glob(os.path.join(self.sdk_output, "var", "lib", "opkg", "*")):
-            bb.utils.movefile(f, native_opkg_state_dir)
+            movefile(f, native_opkg_state_dir)
 
-        bb.utils.remove(os.path.join(self.sdk_output, "var"), True)
+        remove(os.path.join(self.sdk_output, "var"), True)
 
 
 class DpkgSdk(Sdk):
@@ -264,7 +265,7 @@ class DpkgSdk(Sdk):
     def _copy_apt_dir_to(self, dst_dir):
         staging_etcdir_native = self.d.getVar("STAGING_ETCDIR_NATIVE", True)
 
-        bb.utils.remove(dst_dir, True)
+        remove(dst_dir, True)
 
         shutil.copytree(os.path.join(staging_etcdir_native, "apt"), dst_dir)
 
@@ -306,11 +307,11 @@ class DpkgSdk(Sdk):
 
         native_dpkg_state_dir = os.path.join(self.sdk_output, self.sdk_native_path,
                                              "var", "lib", "dpkg")
-        bb.utils.mkdirhier(native_dpkg_state_dir)
+        mkdirhier(native_dpkg_state_dir)
         for f in glob.glob(os.path.join(self.sdk_output, "var", "lib", "dpkg", "*")):
-            bb.utils.movefile(f, native_dpkg_state_dir)
+            movefile(f, native_dpkg_state_dir)
+        remove(os.path.join(self.sdk_output, "var"), True)
 
-        bb.utils.remove(os.path.join(self.sdk_output, "var"), True)
 
 
 def sdk_list_installed_packages(d, target, format=None, rootfs_dir=None):
@@ -345,5 +346,34 @@ def populate_sdk(d, manifest_dir=None):
     os.environ.clear()
     os.environ.update(env_bkp)
 
+def movefile(sourcefile, destdir):
+    try:
+        # FIXME: this check of movefile's return code to None should be
+        # fixed within the function to use only exceptions to signal when
+        # something goes wrong
+        if (bb.utils.movefile(sourcefile, destdir) == None):
+            raise OSError("moving %s to %s failed"
+                    %(sourcefile, destdir))
+    #FIXME: using umbrella exc catching because bb.utils method raises it
+    except Exception as e:
+        bb.debug(1, "printing the stack trace\n %s" %traceback.format_exc())
+        bb.error("unable to place %s in final SDK location" % sourcefile)
+
+def mkdirhier(dirpath):
+    try:
+        bb.utils.mkdirhier(dirpath)
+    except OSError as e:
+        bb.debug(1, "printing the stack trace\n %s" %traceback.format_exc())
+        bb.fatal("cannot make dir for SDK: %s" % dirpath)
+
+def remove(path, recurse=False):
+    try:
+        bb.utils.remove(path, recurse)
+    #FIXME: using umbrella exc catching because bb.utils method raises it
+    except Exception as e:
+        bb.debug(1, "printing the stack trace\n %s" %traceback.format_exc())
+        bb.warn("cannot remove SDK dir: %s" % path)
+
+
 if __name__ == "__main__":
     pass
-- 
2.4.3



^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-10-19 15:28 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-15 17:27 [PATCH] populate SDK: prepare calling of bb.utils for exceptions Benjamin Esquivel
2015-10-16  2:15 ` Christopher Larson
2015-10-16 20:28   ` Benjamin Esquivel
2015-10-16 21:50   ` [PATCH V2] " Benjamin Esquivel
2015-10-19  5:27     ` Mike Looijmans
2015-10-19 15:28       ` [PATCH V3] " Benjamin Esquivel
2015-10-16  7:00 ` [PATCH] " Mike Looijmans
2015-10-16 20:27   ` Benjamin Esquivel
2015-10-16 10:01 ` Richard Purdie
2015-10-16 20:18   ` Benjamin Esquivel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox