Re: [OE-core] [PATCH] staging: remove hard-coded values from _FIXMEs

2018-10-29 Thread richard . purdie
On Wed, 2018-09-19 at 15:08 +0200, Andrej Valek wrote:
> Hi Richard,
> 
> I have sent this patch, because of modification this replacement.
> What
> about adding TMPDIR into this hard-coded array? I think, even if it
> isn't added into EXTRA_STAGING_FIXMES it shouldn't influence default
> behavior.

The reason I wasn't keen on the patch is exactly because people will
add TMPDIR into it and that can hide a number of problems which should
really be better solved in other ways. I didn't want this to become a
workaround for fixing things in the recipes.

So yes, I understand why you might want to do this, I'm still not
convinced its a good idea. There was a good reason this was hard coded
but I still haven't had the time to look more into exactly why that was
:(.

Cheers,

Richard


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


Re: [OE-core] [PATCH] staging: remove hard-coded values from _FIXMEs

2018-10-29 Thread Valek, Andrej
Hi Richard,

Di you have some time to think about it?

Regards,
Andrej

-Original Message-
From: Andrej Valek [mailto:andrej.va...@siemens.com] 
Sent: 19. septembra 2018 15:09
To: Richard Purdie; openembedded-core@lists.openembedded.org
Subject: Re: [OE-core] [PATCH] staging: remove hard-coded values from _FIXMEs

Hi Richard,

I have sent this patch, because of modification this replacement. What about 
adding TMPDIR into this hard-coded array? I think, even if it isn't added into 
EXTRA_STAGING_FIXMES it shouldn't influence default behavior.

Regards,

Andrej

On 9/18/18 2:03 PM, Richard Purdie wrote:
> On Mon, 2018-09-17 at 15:41 +0200, Andrej Valek wrote:
>> Let users to override these values in their layers and could match 
>> them with values in EXTRA_STAGING_FIXMES.
>>
>> Signed-off-by: Andrej Valek 
>> ---
>>  meta/classes/staging.bbclass | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/meta/classes/staging.bbclass 
>> b/meta/classes/staging.bbclass index 84e13bab59..6db501dac1 100644
>> --- a/meta/classes/staging.bbclass
>> +++ b/meta/classes/staging.bbclass
>> @@ -127,6 +127,8 @@ python do_populate_sysroot_setscene () {  }  
>> addtask do_populate_sysroot_setscene
>>  
>> +SYSROOT_STAGING_FIXMES ?= "COMPONENTS_DIR HOSTTOOLS_DIR PKGDATA_DIR
>> PSEUDO_LOCALSTATEDIR LOGFIFO"
>> +
>>  def staging_copyfile(c, target, dest, postinsts, seendirs):
>>  import errno
>>  
>> @@ -167,7 +169,7 @@ def staging_processfixme(fixme, target, 
>> recipesysroot, recipesysrootnative, d):
>>  if not fixme:
>>  return
>>  cmd = "sed -e 's:^[^/]*/:%s/:g' %s | xargs sed -i -e 
>> 's:FIXMESTAGINGDIRTARGET:%s:g; s:FIXMESTAGINGDIRHOST:%s:g'" % 
>> (target, " ".join(fixme), recipesysroot, recipesysrootnative)
>> -for fixmevar in ['COMPONENTS_DIR', 'HOSTTOOLS_DIR',
>> 'PKGDATA_DIR', 'PSEUDO_LOCALSTATEDIR', 'LOGFIFO']:
>> +for fixmevar in d.getVar("SYSROOT_STAGING_FIXMES").split():
>>  fixme_path = d.getVar(fixmevar)
>>  cmd += " -e 's:FIXME_%s:%s:g'" % (fixmevar, fixme_path)
>>  bb.debug(2, cmd)
> 
> I think this was deliberately left this way rather than letting users 
> override it as the scope issues around this are not obvious and making 
> it a variable gives users expectations which may not be met.
> 
> I'm going from memory with jetlag but I think that this variable would 
> not work from recipe context, you'd have to do it in global scope and 
> changing this in global scope for everything is a pretty serious 
> change.
> 
> The reason is that it can get called when building any recipe sysroot 
> so the datastore isn't to context of the original creator.
> 
> Cheers,
> 
> Richard
> 
-- 
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core


Re: [OE-core] [PATCH] staging: remove hard-coded values from _FIXMEs

2018-09-19 Thread Andrej Valek
Hi Richard,

I have sent this patch, because of modification this replacement. What
about adding TMPDIR into this hard-coded array? I think, even if it
isn't added into EXTRA_STAGING_FIXMES it shouldn't influence default
behavior.

Regards,

Andrej

On 9/18/18 2:03 PM, Richard Purdie wrote:
> On Mon, 2018-09-17 at 15:41 +0200, Andrej Valek wrote:
>> Let users to override these values in their layers and could match
>> them
>> with values in EXTRA_STAGING_FIXMES.
>>
>> Signed-off-by: Andrej Valek 
>> ---
>>  meta/classes/staging.bbclass | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/meta/classes/staging.bbclass
>> b/meta/classes/staging.bbclass
>> index 84e13bab59..6db501dac1 100644
>> --- a/meta/classes/staging.bbclass
>> +++ b/meta/classes/staging.bbclass
>> @@ -127,6 +127,8 @@ python do_populate_sysroot_setscene () {
>>  }
>>  addtask do_populate_sysroot_setscene
>>  
>> +SYSROOT_STAGING_FIXMES ?= "COMPONENTS_DIR HOSTTOOLS_DIR PKGDATA_DIR
>> PSEUDO_LOCALSTATEDIR LOGFIFO"
>> +
>>  def staging_copyfile(c, target, dest, postinsts, seendirs):
>>  import errno
>>  
>> @@ -167,7 +169,7 @@ def staging_processfixme(fixme, target,
>> recipesysroot, recipesysrootnative, d):
>>  if not fixme:
>>  return
>>  cmd = "sed -e 's:^[^/]*/:%s/:g' %s | xargs sed -i -e
>> 's:FIXMESTAGINGDIRTARGET:%s:g; s:FIXMESTAGINGDIRHOST:%s:g'" %
>> (target, " ".join(fixme), recipesysroot, recipesysrootnative)
>> -for fixmevar in ['COMPONENTS_DIR', 'HOSTTOOLS_DIR',
>> 'PKGDATA_DIR', 'PSEUDO_LOCALSTATEDIR', 'LOGFIFO']:
>> +for fixmevar in d.getVar("SYSROOT_STAGING_FIXMES").split():
>>  fixme_path = d.getVar(fixmevar)
>>  cmd += " -e 's:FIXME_%s:%s:g'" % (fixmevar, fixme_path)
>>  bb.debug(2, cmd)
> 
> I think this was deliberately left this way rather than letting users
> override it as the scope issues around this are not obvious and making
> it a variable gives users expectations which may not be met.
> 
> I'm going from memory with jetlag but I think that this variable would
> not work from recipe context, you'd have to do it in global scope and
> changing this in global scope for everything is a pretty serious
> change.
> 
> The reason is that it can get called when building any recipe sysroot
> so the datastore isn't to context of the original creator.
> 
> Cheers,
> 
> Richard
> 
-- 
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core


Re: [OE-core] [PATCH] staging: remove hard-coded values from _FIXMEs

2018-09-18 Thread Richard Purdie
On Mon, 2018-09-17 at 15:41 +0200, Andrej Valek wrote:
> Let users to override these values in their layers and could match
> them
> with values in EXTRA_STAGING_FIXMES.
> 
> Signed-off-by: Andrej Valek 
> ---
>  meta/classes/staging.bbclass | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/meta/classes/staging.bbclass
> b/meta/classes/staging.bbclass
> index 84e13bab59..6db501dac1 100644
> --- a/meta/classes/staging.bbclass
> +++ b/meta/classes/staging.bbclass
> @@ -127,6 +127,8 @@ python do_populate_sysroot_setscene () {
>  }
>  addtask do_populate_sysroot_setscene
>  
> +SYSROOT_STAGING_FIXMES ?= "COMPONENTS_DIR HOSTTOOLS_DIR PKGDATA_DIR
> PSEUDO_LOCALSTATEDIR LOGFIFO"
> +
>  def staging_copyfile(c, target, dest, postinsts, seendirs):
>  import errno
>  
> @@ -167,7 +169,7 @@ def staging_processfixme(fixme, target,
> recipesysroot, recipesysrootnative, d):
>  if not fixme:
>  return
>  cmd = "sed -e 's:^[^/]*/:%s/:g' %s | xargs sed -i -e
> 's:FIXMESTAGINGDIRTARGET:%s:g; s:FIXMESTAGINGDIRHOST:%s:g'" %
> (target, " ".join(fixme), recipesysroot, recipesysrootnative)
> -for fixmevar in ['COMPONENTS_DIR', 'HOSTTOOLS_DIR',
> 'PKGDATA_DIR', 'PSEUDO_LOCALSTATEDIR', 'LOGFIFO']:
> +for fixmevar in d.getVar("SYSROOT_STAGING_FIXMES").split():
>  fixme_path = d.getVar(fixmevar)
>  cmd += " -e 's:FIXME_%s:%s:g'" % (fixmevar, fixme_path)
>  bb.debug(2, cmd)

I think this was deliberately left this way rather than letting users
override it as the scope issues around this are not obvious and making
it a variable gives users expectations which may not be met.

I'm going from memory with jetlag but I think that this variable would
not work from recipe context, you'd have to do it in global scope and
changing this in global scope for everything is a pretty serious
change.

The reason is that it can get called when building any recipe sysroot
so the datastore isn't to context of the original creator.

Cheers,

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


[OE-core] [PATCH] staging: remove hard-coded values from _FIXMEs

2018-09-17 Thread Andrej Valek
Let users to override these values in their layers and could match them
with values in EXTRA_STAGING_FIXMES.

Signed-off-by: Andrej Valek 
---
 meta/classes/staging.bbclass | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/meta/classes/staging.bbclass b/meta/classes/staging.bbclass
index 84e13bab59..6db501dac1 100644
--- a/meta/classes/staging.bbclass
+++ b/meta/classes/staging.bbclass
@@ -127,6 +127,8 @@ python do_populate_sysroot_setscene () {
 }
 addtask do_populate_sysroot_setscene
 
+SYSROOT_STAGING_FIXMES ?= "COMPONENTS_DIR HOSTTOOLS_DIR PKGDATA_DIR 
PSEUDO_LOCALSTATEDIR LOGFIFO"
+
 def staging_copyfile(c, target, dest, postinsts, seendirs):
 import errno
 
@@ -167,7 +169,7 @@ def staging_processfixme(fixme, target, recipesysroot, 
recipesysrootnative, d):
 if not fixme:
 return
 cmd = "sed -e 's:^[^/]*/:%s/:g' %s | xargs sed -i -e 
's:FIXMESTAGINGDIRTARGET:%s:g; s:FIXMESTAGINGDIRHOST:%s:g'" % (target, " 
".join(fixme), recipesysroot, recipesysrootnative)
-for fixmevar in ['COMPONENTS_DIR', 'HOSTTOOLS_DIR', 'PKGDATA_DIR', 
'PSEUDO_LOCALSTATEDIR', 'LOGFIFO']:
+for fixmevar in d.getVar("SYSROOT_STAGING_FIXMES").split():
 fixme_path = d.getVar(fixmevar)
 cmd += " -e 's:FIXME_%s:%s:g'" % (fixmevar, fixme_path)
 bb.debug(2, cmd)
-- 
2.11.0

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