Re: [OE-core] [PATCH 7/8] populate_sdk_ext.bbclass: remove the try...finally

[Robert Yang]

On 03/13/2018 10:57 PM, Richard Purdie wrote:

On Tue, 2018-03-13 at 11:24 +0800, Robert Yang wrote:

The "sdkbasepath + '/conf/local.conf.bak" doesn't exist when
"oe.copy_buildsystem.check_sstate_task_list()" fails, then
os.replace() would
raise FileNotFoundError, which overcomes the real error. Keep the
error status
makes debug easier, so remove the try..finally.


I don't think this patch is quite right. If there is a failure we
*must* make sure local.conf is restored, that is important. The code
should probably keep the try/finally but make the replace conditional
on the file existing.


Thanks, updated in the repo:

Author: Robert Yang 
Date:   Wed Mar 14 10:01:51 2018 +0800

populate_sdk_ext.bbclass: only replace local.conf when local.conf.bak exists

The "sdkbasepath + '/conf/local.conf.bak'" doesn't exist when
"oe.copy_buildsystem.check_sstate_task_list()" fails since the sdkbasepath 
had
been renamed to temp_sdkbasepath, but can't rename back when
oe.copy_buildsystem.check_sstate_task_list() fails, then os.replace() would
raise FileNotFoundError, which overcomes the real error. Only replace
local.conf when local_conf.bak exists can make the debug easier.

Signed-off-by: Robert Yang 

diff --git a/meta/classes/populate_sdk_ext.bbclass 
b/meta/classes/populate_sdk_ext.bbclass

index 057c495..fa24fc4 100644
--- a/meta/classes/populate_sdk_ext.bbclass
+++ b/meta/classes/populate_sdk_ext.bbclass
@@ -141,9 +141,11 @@ def create_filtered_tasklist(d, sdkbasepath, tasklistfile, 
conf_initpath):

 import oe.copy_buildsystem

 # Create a temporary build directory that we can pass to the env setup 
script
-shutil.copyfile(sdkbasepath + '/conf/local.conf', sdkbasepath + 
'/conf/local.conf.bak')

+local_conf = sdkbasepath + '/conf/local.conf'
+local_conf_bak = local_conf + '.bak'
+shutil.copyfile(local_conf, local_conf_bak)
 try:
-with open(sdkbasepath + '/conf/local.conf', 'a') as f:
+with open(local_conf, 'a') as f:
 # Force the use of sstate from the build system
 f.write('\nSSTATE_DIR_forcevariable = "%s"\n' % 
d.getVar('SSTATE_DIR'))
 f.write('SSTATE_MIRRORS_forcevariable = "file://universal/(.*) 
file://universal-4.9/\\1 file://universal-4.9/(.*) file://universal-4.8/\\1"\n')
@@ -178,7 +180,8 @@ def create_filtered_tasklist(d, sdkbasepath, tasklistfile, 
conf_initpath):

 # will effectively do
 clean_esdk_builddir(d, sdkbasepath)
 finally:
-os.replace(sdkbasepath + '/conf/local.conf.bak', sdkbasepath + 
'/conf/local.conf')

+if os.path.exists(local_conf_bak):
+os.replace(local_conf_bak, local_conf)

 python copy_buildsystem () {
 import re

// Robert




Cheers,

Richard


--
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [OE-core] [PATCH 7/8] populate_sdk_ext.bbclass: remove the try...finally

[Richard Purdie]

On Tue, 2018-03-13 at 11:24 +0800, Robert Yang wrote:
> The "sdkbasepath + '/conf/local.conf.bak" doesn't exist when
> "oe.copy_buildsystem.check_sstate_task_list()" fails, then
> os.replace() would
> raise FileNotFoundError, which overcomes the real error. Keep the
> error status
> makes debug easier, so remove the try..finally.

I don't think this patch is quite right. If there is a failure we
*must* make sure local.conf is restored, that is important. The code
should probably keep the try/finally but make the replace conditional
on the file existing.

Cheers,

Richard
-- 
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core

[OE-core] [PATCH 7/8] populate_sdk_ext.bbclass: remove the try...finally

[Robert Yang]

The "sdkbasepath + '/conf/local.conf.bak" doesn't exist when
"oe.copy_buildsystem.check_sstate_task_list()" fails, then os.replace() would
raise FileNotFoundError, which overcomes the real error. Keep the error status
makes debug easier, so remove the try..finally.

Signed-off-by: Robert Yang 
---
 meta/classes/populate_sdk_ext.bbclass | 70 +--
 1 file changed, 34 insertions(+), 36 deletions(-)

diff --git a/meta/classes/populate_sdk_ext.bbclass 
b/meta/classes/populate_sdk_ext.bbclass
index 057c495..43823bf 100644
--- a/meta/classes/populate_sdk_ext.bbclass
+++ b/meta/classes/populate_sdk_ext.bbclass
@@ -142,43 +142,41 @@ def create_filtered_tasklist(d, sdkbasepath, 
tasklistfile, conf_initpath):
 
 # Create a temporary build directory that we can pass to the env setup 
script
 shutil.copyfile(sdkbasepath + '/conf/local.conf', sdkbasepath + 
'/conf/local.conf.bak')
+with open(sdkbasepath + '/conf/local.conf', 'a') as f:
+# Force the use of sstate from the build system
+f.write('\nSSTATE_DIR_forcevariable = "%s"\n' % d.getVar('SSTATE_DIR'))
+f.write('SSTATE_MIRRORS_forcevariable = "file://universal/(.*) 
file://universal-4.9/\\1 file://universal-4.9/(.*) file://universal-4.8/\\1"\n')
+# Ensure TMPDIR is the default so that clean_esdk_builddir() can 
delete it
+f.write('TMPDIR_forcevariable = "${TOPDIR}/tmp"\n')
+f.write('TCLIBCAPPEND_forcevariable = ""\n')
+# Drop uninative if the build isn't using it (or else NATIVELSBSTRING 
will
+# be different and we won't be able to find our native sstate)
+if not bb.data.inherits_class('uninative', d):
+f.write('INHERIT_remove = "uninative"\n')
+
+# Unfortunately the default SDKPATH (or even a custom value) may contain 
characters that bitbake
+# will not allow in its COREBASE path, so we need to rename the directory 
temporarily
+temp_sdkbasepath = d.getVar('SDK_OUTPUT') + '/tmp-renamed-sdk'
+# Delete any existing temp dir
 try:
-with open(sdkbasepath + '/conf/local.conf', 'a') as f:
-# Force the use of sstate from the build system
-f.write('\nSSTATE_DIR_forcevariable = "%s"\n' % 
d.getVar('SSTATE_DIR'))
-f.write('SSTATE_MIRRORS_forcevariable = "file://universal/(.*) 
file://universal-4.9/\\1 file://universal-4.9/(.*) file://universal-4.8/\\1"\n')
-# Ensure TMPDIR is the default so that clean_esdk_builddir() can 
delete it
-f.write('TMPDIR_forcevariable = "${TOPDIR}/tmp"\n')
-f.write('TCLIBCAPPEND_forcevariable = ""\n')
-# Drop uninative if the build isn't using it (or else 
NATIVELSBSTRING will
-# be different and we won't be able to find our native sstate)
-if not bb.data.inherits_class('uninative', d):
-f.write('INHERIT_remove = "uninative"\n')
-
-# Unfortunately the default SDKPATH (or even a custom value) may 
contain characters that bitbake
-# will not allow in its COREBASE path, so we need to rename the 
directory temporarily
-temp_sdkbasepath = d.getVar('SDK_OUTPUT') + '/tmp-renamed-sdk'
-# Delete any existing temp dir
-try:
-shutil.rmtree(temp_sdkbasepath)
-except FileNotFoundError:
-pass
-os.rename(sdkbasepath, temp_sdkbasepath)
-cmdprefix = '. %s .; ' % conf_initpath
-logfile = d.getVar('WORKDIR') + '/tasklist_bb_log.txt'
-try:
-oe.copy_buildsystem.check_sstate_task_list(d, 
get_sdk_install_targets(d), tasklistfile, cmdprefix=cmdprefix, 
cwd=temp_sdkbasepath, logfile=logfile)
-except bb.process.ExecutionError as e:
-msg = 'Failed to generate filtered task list for extensible 
SDK:\n%s' %  e.stdout.rstrip()
-if 'attempted to execute unexpectedly and should have been 
setscened' in e.stdout:
-msg += '\n--\n\nNOTE: "attempted to execute 
unexpectedly and should have been setscened" errors indicate this may be caused 
by missing sstate artifacts that were likely produced in earlier builds, but 
have been subsequently deleted for some reason.\n'
-bb.fatal(msg)
-os.rename(temp_sdkbasepath, sdkbasepath)
-# Clean out residue of running bitbake, which check_sstate_task_list()
-# will effectively do
-clean_esdk_builddir(d, sdkbasepath)
-finally:
-os.replace(sdkbasepath + '/conf/local.conf.bak', sdkbasepath + 
'/conf/local.conf')
+shutil.rmtree(temp_sdkbasepath)
+except FileNotFoundError:
+pass
+os.rename(sdkbasepath, temp_sdkbasepath)
+cmdprefix = '. %s .; ' % conf_initpath
+logfile = d.getVar('WORKDIR') + '/tasklist_bb_log.txt'
+try:
+oe.copy_buildsystem.check_sstate_task_list(d, 
get_sdk_install_targets(d), tasklistfile, cmdprefix=cmdprefix, 
cwd=temp_sdkbasepath, logfile=logfile)
+except