Re: [OE-core] [PATCH V3 09/11] populate_sdk_ext.bbclass: add ESDK_MANIFEST_EXCLUDES

2018-07-12 Thread Robert Yang

HI Paul,

On 07/06/2018 06:12 PM, Paul Eggleton wrote:

Hi Robert / Chen

On Wednesday, 6 June 2018 4:54:44 AM CEST Robert Yang wrote:

From: Chen Qi 

Add ESDK_MANIFEST_EXCLUDES to enable excluding items in sdk-conf-manifest.

By default, files under conf/ are all added to sdk-conf-manifest, as the
manifest file is set to 'conf/*'.

However, there are situations where some configuration files under conf/
directory are not intended to be added to sdk-conf-manifest, thus adding
ESDK_MANIFEST_EXCLUDES to enable users to do this.

This variable takes the form of glob matching.
e.g.
ESDK_MANIFEST_EXCLUDES = "conf/autogen*"
This would exclude all files under conf/ starting with 'autogen' from
sdk-conf-manifest.


This patch (and 05/11, SDK_LAYERS_EXCLUDE*) worry me a little in that they
have the potential to break the resulting eSDK or make it behave in a manner
that is different from the build system that produced it. Having said that
it's going to be reasonably clear to the user what's happened, assuming they
remember they set these variables and in any case I don't expect these are
going to be set by many people. Accordingly I won't object to these patches,
but could you please add a warning about this issue to the commit message for
both? We'd also better ensure they get documented with similar warnings.


I think that a bb.note() is enough for it, we don't set them by default,
but set by users, so they should know something is excluded. So I will add
a bb.note for it:

bb.note('Exclude %s since it is in SDK_CONF_MANIFEST_EXCLUDE' % fn)



Apart from that I'd like to see a different name for the variable here - we
don't use the prefix ESDK_ anywhere else, so perhaps SDK_CONF_MANIFEST_EXCLUDE
?


Thanks, I will update to SDK_CONF_MANIFEST_EXCLUDE in V4.

// Robert



Cheers,
Paul




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


Re: [OE-core] [PATCH V3 09/11] populate_sdk_ext.bbclass: add ESDK_MANIFEST_EXCLUDES

2018-07-06 Thread Robert Yang



On 07/06/2018 06:12 PM, Paul Eggleton wrote:

Hi Robert / Chen

On Wednesday, 6 June 2018 4:54:44 AM CEST Robert Yang wrote:

From: Chen Qi 

Add ESDK_MANIFEST_EXCLUDES to enable excluding items in sdk-conf-manifest.

By default, files under conf/ are all added to sdk-conf-manifest, as the
manifest file is set to 'conf/*'.

However, there are situations where some configuration files under conf/
directory are not intended to be added to sdk-conf-manifest, thus adding
ESDK_MANIFEST_EXCLUDES to enable users to do this.

This variable takes the form of glob matching.
e.g.
ESDK_MANIFEST_EXCLUDES = "conf/autogen*"
This would exclude all files under conf/ starting with 'autogen' from
sdk-conf-manifest.


This patch (and 05/11, SDK_LAYERS_EXCLUDE*) worry me a little in that they
have the potential to break the resulting eSDK or make it behave in a manner
that is different from the build system that produced it. Having said that
it's going to be reasonably clear to the user what's happened, assuming they
remember they set these variables and in any case I don't expect these are
going to be set by many people. Accordingly I won't object to these patches,
but could you please add a warning about this issue to the commit message for
both? We'd also better ensure they get documented with similar warnings.

Apart from that I'd like to see a different name for the variable here - we
don't use the prefix ESDK_ anywhere else, so perhaps SDK_CONF_MANIFEST_EXCLUDE
?


Thanks, make sense, will update it next week.

// Robert



Cheers,
Paul




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


Re: [OE-core] [PATCH V3 09/11] populate_sdk_ext.bbclass: add ESDK_MANIFEST_EXCLUDES

2018-07-06 Thread Paul Eggleton
Hi Robert / Chen

On Wednesday, 6 June 2018 4:54:44 AM CEST Robert Yang wrote:
> From: Chen Qi 
> 
> Add ESDK_MANIFEST_EXCLUDES to enable excluding items in sdk-conf-manifest.
> 
> By default, files under conf/ are all added to sdk-conf-manifest, as the
> manifest file is set to 'conf/*'.
> 
> However, there are situations where some configuration files under conf/
> directory are not intended to be added to sdk-conf-manifest, thus adding
> ESDK_MANIFEST_EXCLUDES to enable users to do this.
> 
> This variable takes the form of glob matching.
> e.g.
> ESDK_MANIFEST_EXCLUDES = "conf/autogen*"
> This would exclude all files under conf/ starting with 'autogen' from
> sdk-conf-manifest.

This patch (and 05/11, SDK_LAYERS_EXCLUDE*) worry me a little in that they 
have the potential to break the resulting eSDK or make it behave in a manner 
that is different from the build system that produced it. Having said that 
it's going to be reasonably clear to the user what's happened, assuming they 
remember they set these variables and in any case I don't expect these are 
going to be set by many people. Accordingly I won't object to these patches, 
but could you please add a warning about this issue to the commit message for 
both? We'd also better ensure they get documented with similar warnings.

Apart from that I'd like to see a different name for the variable here - we 
don't use the prefix ESDK_ anywhere else, so perhaps SDK_CONF_MANIFEST_EXCLUDE 
?

Cheers,
Paul



-- 

Paul Eggleton
Intel Open Source Technology Centre


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


[OE-core] [PATCH V3 09/11] populate_sdk_ext.bbclass: add ESDK_MANIFEST_EXCLUDES

2018-06-05 Thread Robert Yang
From: Chen Qi 

Add ESDK_MANIFEST_EXCLUDES to enable excluding items in sdk-conf-manifest.

By default, files under conf/ are all added to sdk-conf-manifest, as the
manifest file is set to 'conf/*'.

However, there are situations where some configuration files under conf/
directory are not intended to be added to sdk-conf-manifest, thus adding
ESDK_MANIFEST_EXCLUDES to enable users to do this.

This variable takes the form of glob matching.
e.g.
ESDK_MANIFEST_EXCLUDES = "conf/autogen*"
This would exclude all files under conf/ starting with 'autogen' from
sdk-conf-manifest.

Signed-off-by: Chen Qi 
---
 meta/classes/populate_sdk_ext.bbclass | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/meta/classes/populate_sdk_ext.bbclass 
b/meta/classes/populate_sdk_ext.bbclass
index e1bba49..017c4b6 100644
--- a/meta/classes/populate_sdk_ext.bbclass
+++ b/meta/classes/populate_sdk_ext.bbclass
@@ -484,12 +484,18 @@ python copy_buildsystem () {
 # sdk_ext_postinst() below) thus the checksum we take here would always
 # be different.
 manifest_file_list = ['conf/*']
+esdk_manifest_excludes = (d.getVar('ESDK_MANIFEST_EXCLUDES') or '').split()
+esdk_manifest_excludes_list = []
+for exclude_item in esdk_manifest_excludes:
+esdk_manifest_excludes_list += glob.glob(os.path.join(baseoutpath, 
exclude_item))
 manifest_file = os.path.join(baseoutpath, 'conf', 'sdk-conf-manifest')
 with open(manifest_file, 'w') as f:
 for item in manifest_file_list:
 for fn in glob.glob(os.path.join(baseoutpath, item)):
 if fn == manifest_file:
 continue
+if fn in esdk_manifest_excludes_list:
+continue
 chksum = bb.utils.sha256_file(fn)
 f.write('%s\t%s\n' % (chksum, os.path.relpath(fn, 
baseoutpath)))
 }
-- 
2.7.4

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