[PATCH] bitbake.conf: Changed PSEUDO_LOCALSTATEDIR assignment to unconditional.

[PATCH] bitbake.conf: Changed PSEUDO_LOCALSTATEDIR assignment to unconditional.

[James Limbouris]

The pseudo executable sets the PSEUDO_LOCALSTATEDIR environment variable
to point to a sysroot location. During an initial cache build, in which
pseudo is not used as it is being built, the PSEUDO_LOCALSTATEDIR is set
to a per-package location by bitbake.conf, and baked into the cache.
If the cache is subsequently invalidated, bitbake may run under pseudo,
and rebuild it with the sysroot location baked into the cache. This
results in all packages using the same, persistent db, even on package
rebuild, which leads to permissions errors. Making the assignment
unconditional corrects this problem.

Signed-off-by: James Limbouris <james@digitalmatter.com.au>
---
 meta/conf/bitbake.conf |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
index 6f0b42c..9a9ab53 100644
--- a/meta/conf/bitbake.conf
+++ b/meta/conf/bitbake.conf
@@ -557,7 +557,7 @@ SRCPV = "${@bb.fetch2.get_srcrev(d)}"
 SRC_URI = "file://${FILE}"
 
 # Use pseudo as the fakeroot implementation
-PSEUDO_LOCALSTATEDIR ?= "${WORKDIR}/pseudo/"
+PSEUDO_LOCALSTATEDIR = "${WORKDIR}/pseudo/"
 FAKEROOTENV = "PSEUDO_PREFIX=${STAGING_DIR_NATIVE}${prefix_native} PSEUDO_LOCALSTATEDIR=${PSEUDO_LOCALSTATEDIR} PSEUDO_PASSWD=${STAGING_DIR_TARGET} PSEUDO_NOSYMLINKEXP=1 PSEUDO_DISABLED=0"
 FAKEROOTDIRS = "${PSEUDO_LOCALSTATEDIR}"
 PREFERRED_PROVIDER_virtual/fakeroot-native ?= "pseudo-native"
-- 
1.7.3.4

Re: [PATCH] bitbake.conf: Changed PSEUDO_LOCALSTATEDIR assignment to unconditional.

[Scott Garman]

On 08/08/2011 07:04 PM, James Limbouris wrote:
> The pseudo executable sets the PSEUDO_LOCALSTATEDIR environment variable
> to point to a sysroot location. During an initial cache build, in which
> pseudo is not used as it is being built, the PSEUDO_LOCALSTATEDIR is set
> to a per-package location by bitbake.conf, and baked into the cache.
> If the cache is subsequently invalidated, bitbake may run under pseudo,
> and rebuild it with the sysroot location baked into the cache. This
> results in all packages using the same, persistent db, even on package
> rebuild, which leads to permissions errors. Making the assignment
> unconditional corrects this problem.

I can't tell if this change would impact the use of useradd.bbclass, 
which also sets PSEUDO_LOCALSTATEDIR in certain circumstances. Mark, can 
you comment on whether this is a valid concern?

Scott

>
> Signed-off-by: James Limbouris<james@digitalmatter.com.au>
> ---
>   meta/conf/bitbake.conf |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
> index 6f0b42c..9a9ab53 100644
> --- a/meta/conf/bitbake.conf
> +++ b/meta/conf/bitbake.conf
> @@ -557,7 +557,7 @@ SRCPV = "${@bb.fetch2.get_srcrev(d)}"
>   SRC_URI = "file://${FILE}"
>
>   # Use pseudo as the fakeroot implementation
> -PSEUDO_LOCALSTATEDIR ?= "${WORKDIR}/pseudo/"
> +PSEUDO_LOCALSTATEDIR = "${WORKDIR}/pseudo/"
>   FAKEROOTENV = "PSEUDO_PREFIX=${STAGING_DIR_NATIVE}${prefix_native} PSEUDO_LOCALSTATEDIR=${PSEUDO_LOCALSTATEDIR} PSEUDO_PASSWD=${STAGING_DIR_TARGET} PSEUDO_NOSYMLINKEXP=1 PSEUDO_DISABLED=0"
>   FAKEROOTDIRS = "${PSEUDO_LOCALSTATEDIR}"
>   PREFERRED_PROVIDER_virtual/fakeroot-native ?= "pseudo-native"


-- 
Scott Garman
Embedded Linux Engineer - Yocto Project
Intel Open Source Technology Center

Re: [PATCH] bitbake.conf: Changed PSEUDO_LOCALSTATEDIR assignment to unconditional.

[Mark Hatle]

On 8/9/11 7:32 PM, Scott Garman wrote:
> On 08/08/2011 07:04 PM, James Limbouris wrote:
>> The pseudo executable sets the PSEUDO_LOCALSTATEDIR environment variable
>> to point to a sysroot location. During an initial cache build, in which
>> pseudo is not used as it is being built, the PSEUDO_LOCALSTATEDIR is set
>> to a per-package location by bitbake.conf, and baked into the cache.
>> If the cache is subsequently invalidated, bitbake may run under pseudo,
>> and rebuild it with the sysroot location baked into the cache. This
>> results in all packages using the same, persistent db, even on package
>> rebuild, which leads to permissions errors. Making the assignment
>> unconditional corrects this problem.
> 
> I can't tell if this change would impact the use of useradd.bbclass, 
> which also sets PSEUDO_LOCALSTATEDIR in certain circumstances. Mark, can 
> you comment on whether this is a valid concern?

I've been trying to figure that out as well.  From what I can tell, as long as
the value is "${WORKDIR}/pseudo", and it's evaluated at use time -- vs
immediately.. we should be good, even with the useradd.bbclass.

(If I remember correctly, the workdir for the useradd still should either be the
package itself, or the sysroot directory.  That should be changing during the
various steps, as necessary.)

Frankly I'm still a little confused as to what is happening differently from the
"=" to the "?=" case.  My understanding is that one should indicate "it's always
this value", and the other is "use this value if one has not already been set."
 Either way we should get the same result, and the same result should end up in
any caches.  If it's not the same, this might be pointing to a different bug --
or my understanding of the processing of the variables is wrong.

So for the purposes of useradd, I don't see a problem here.. I also don't see
any issues with the general case.. but I do wonder why it's needed for the cache
to work properly.

--Mark

> Scott
> 
>>
>> Signed-off-by: James Limbouris<james@digitalmatter.com.au>
>> ---
>>   meta/conf/bitbake.conf |    2 +-
>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
>> index 6f0b42c..9a9ab53 100644
>> --- a/meta/conf/bitbake.conf
>> +++ b/meta/conf/bitbake.conf
>> @@ -557,7 +557,7 @@ SRCPV = "${@bb.fetch2.get_srcrev(d)}"
>>   SRC_URI = "file://${FILE}"
>>
>>   # Use pseudo as the fakeroot implementation
>> -PSEUDO_LOCALSTATEDIR ?= "${WORKDIR}/pseudo/"
>> +PSEUDO_LOCALSTATEDIR = "${WORKDIR}/pseudo/"
>>   FAKEROOTENV = "PSEUDO_PREFIX=${STAGING_DIR_NATIVE}${prefix_native} PSEUDO_LOCALSTATEDIR=${PSEUDO_LOCALSTATEDIR} PSEUDO_PASSWD=${STAGING_DIR_TARGET} PSEUDO_NOSYMLINKEXP=1 PSEUDO_DISABLED=0"
>>   FAKEROOTDIRS = "${PSEUDO_LOCALSTATEDIR}"
>>   PREFERRED_PROVIDER_virtual/fakeroot-native ?= "pseudo-native"
> 
> 

Re: [PATCH] bitbake.conf: Changed PSEUDO_LOCALSTATEDIR assignment to unconditional.

[James Limbouris]

On Wednesday, 10 August 2011 9:55 AM, Mark Hatle wrote:
> On 8/9/11 7:32 PM, Scott Garman wrote:
> > On 08/08/2011 07:04 PM, James Limbouris wrote:
> >> The pseudo executable sets the PSEUDO_LOCALSTATEDIR environment
> >> variable to point to a sysroot location. During an initial cache
> >> build, in which pseudo is not used as it is being built, the
> >> PSEUDO_LOCALSTATEDIR is set to a per-package location by bitbake.conf,
> and baked into the cache.
> >> If the cache is subsequently invalidated, bitbake may run under
> >> pseudo, and rebuild it with the sysroot location baked into the
> >> cache. This results in all packages using the same, persistent db,
> >> even on package rebuild, which leads to permissions errors. Making
> >> the assignment unconditional corrects this problem.
> >
> > I can't tell if this change would impact the use of useradd.bbclass,
> > which also sets PSEUDO_LOCALSTATEDIR in certain circumstances. Mark,
> > can you comment on whether this is a valid concern?
> 
> I've been trying to figure that out as well.  From what I can tell, as long as the
> value is "${WORKDIR}/pseudo", and it's evaluated at use time -- vs
> immediately.. we should be good, even with the useradd.bbclass.
> 
> (If I remember correctly, the workdir for the useradd still should either be
> the package itself, or the sysroot directory.  That should be changing during
> the various steps, as necessary.)
> 
> Frankly I'm still a little confused as to what is happening differently from the
> "=" to the "?=" case.  My understanding is that one should indicate "it's
> always this value", and the other is "use this value if one has not already
> been set."
>  Either way we should get the same result, and the same result should end
> up in any caches.  If it's not the same, this might be pointing to a different
> bug -- or my understanding of the processing of the variables is wrong.
> 
> So for the purposes of useradd, I don't see a problem here.. I also don't see
> any issues with the general case.. but I do wonder why it's needed for the
> cache to work properly.
> 

Hi,

The problem is that the pseudo executable sets the environment variable
before starting its argument (bitbake) up, but we do not always use pseudo
to start bitbake. So sometimes the variable is set to the sysroot, and sometimes
it is set (by the conditional assignment in bitbake.conf) to the workdir.

Usually it is set to the workdir while building the cache, but when the cache is
invalidated, the wrapper script most often uses pseudo, setting it to the sysroot.

James

Re: [PATCH] bitbake.conf: Changed PSEUDO_LOCALSTATEDIR assignment to unconditional.

[Richard Purdie]

On Wed, 2011-08-10 at 02:06 +0000, James Limbouris wrote:
> On Wednesday, 10 August 2011 9:55 AM, Mark Hatle wrote:
> > On 8/9/11 7:32 PM, Scott Garman wrote:
> > > On 08/08/2011 07:04 PM, James Limbouris wrote:
> > >> The pseudo executable sets the PSEUDO_LOCALSTATEDIR environment
> > >> variable to point to a sysroot location. During an initial cache
> > >> build, in which pseudo is not used as it is being built, the
> > >> PSEUDO_LOCALSTATEDIR is set to a per-package location by bitbake.conf,
> > and baked into the cache.
> > >> If the cache is subsequently invalidated, bitbake may run under
> > >> pseudo, and rebuild it with the sysroot location baked into the
> > >> cache. This results in all packages using the same, persistent db,
> > >> even on package rebuild, which leads to permissions errors. Making
> > >> the assignment unconditional corrects this problem.
> > >
> > > I can't tell if this change would impact the use of useradd.bbclass,
> > > which also sets PSEUDO_LOCALSTATEDIR in certain circumstances. Mark,
> > > can you comment on whether this is a valid concern?
> > 
> > I've been trying to figure that out as well.  From what I can tell, as long as the
> > value is "${WORKDIR}/pseudo", and it's evaluated at use time -- vs
> > immediately.. we should be good, even with the useradd.bbclass.
> > 
> > (If I remember correctly, the workdir for the useradd still should either be
> > the package itself, or the sysroot directory.  That should be changing during
> > the various steps, as necessary.)
> > 
> > Frankly I'm still a little confused as to what is happening differently from the
> > "=" to the "?=" case.  My understanding is that one should indicate "it's
> > always this value", and the other is "use this value if one has not already
> > been set."
> >  Either way we should get the same result, and the same result should end
> > up in any caches.  If it's not the same, this might be pointing to a different
> > bug -- or my understanding of the processing of the variables is wrong.
> > 
> > So for the purposes of useradd, I don't see a problem here.. I also don't see
> > any issues with the general case.. but I do wonder why it's needed for the
> > cache to work properly.
> > 
> 
> Hi,
> 
> The problem is that the pseudo executable sets the environment variable
> before starting its argument (bitbake) up, but we do not always use pseudo
> to start bitbake. So sometimes the variable is set to the sysroot, and sometimes
> it is set (by the conditional assignment in bitbake.conf) to the workdir.

Something here isn't adding up correctly to me.

bitbake should not be seeing any PSEUDO_LOCALSTATEDIR from the pseudo
binary since its not a whitelisted environment variable. The ?= should
therefore always being assigned as there wouldn't be a previous version
of the variable.

I'd like to understand what value the variable is taking (which would
stop the ?= applying).

Are you using BB_PRESERVE_ENV ?

Cheers,

Richard

> Usually it is set to the workdir while building the cache, but when the cache is
> invalidated, the wrapper script most often uses pseudo, setting it to the sysroot.
> 
> James

Re: [PATCH] bitbake.conf: Changed PSEUDO_LOCALSTATEDIR assignment to unconditional.

[James Limbouris]

On Wed, 10 August 2011 8:06 PM, Richard Purdie wrote:
> On Wed, 2011-08-10 at 02:06 +0000, James Limbouris wrote:
> > On Wednesday, 10 August 2011 9:55 AM, Mark Hatle wrote:
> > > On 8/9/11 7:32 PM, Scott Garman wrote:
> > > > On 08/08/2011 07:04 PM, James Limbouris wrote:
> > > >> The pseudo executable sets the PSEUDO_LOCALSTATEDIR
> environment
> > > >> variable to point to a sysroot location. During an initial cache
> > > >> build, in which pseudo is not used as it is being built, the
> > > >> PSEUDO_LOCALSTATEDIR is set to a per-package location by
> > > >> bitbake.conf,
> > > and baked into the cache.
> > > >> If the cache is subsequently invalidated, bitbake may run under
> > > >> pseudo, and rebuild it with the sysroot location baked into the
> > > >> cache. This results in all packages using the same, persistent
> > > >> db, even on package rebuild, which leads to permissions errors.
> > > >> Making the assignment unconditional corrects this problem.
> > > >
> > > > I can't tell if this change would impact the use of
> > > > useradd.bbclass, which also sets PSEUDO_LOCALSTATEDIR in certain
> > > > circumstances. Mark, can you comment on whether this is a valid
> concern?
> > >
> > > I've been trying to figure that out as well.  From what I can tell,
> > > as long as the value is "${WORKDIR}/pseudo", and it's evaluated at
> > > use time -- vs immediately.. we should be good, even with the
> useradd.bbclass.
> > >
> > > (If I remember correctly, the workdir for the useradd still should
> > > either be the package itself, or the sysroot directory.  That should
> > > be changing during the various steps, as necessary.)
> > >
> > > Frankly I'm still a little confused as to what is happening
> > > differently from the "=" to the "?=" case.  My understanding is that
> > > one should indicate "it's always this value", and the other is "use
> > > this value if one has not already been set."
> > >  Either way we should get the same result, and the same result
> > > should end up in any caches.  If it's not the same, this might be
> > > pointing to a different bug -- or my understanding of the processing of
> the variables is wrong.
> > >
> > > So for the purposes of useradd, I don't see a problem here.. I also
> > > don't see any issues with the general case.. but I do wonder why
> > > it's needed for the cache to work properly.
> > >
> >
> > Hi,
> >
> > The problem is that the pseudo executable sets the environment
> > variable before starting its argument (bitbake) up, but we do not
> > always use pseudo to start bitbake. So sometimes the variable is set
> > to the sysroot, and sometimes it is set (by the conditional assignment in
> bitbake.conf) to the workdir.
> 
> Something here isn't adding up correctly to me.
> 
> bitbake should not be seeing any PSEUDO_LOCALSTATEDIR from the pseudo
> binary since its not a whitelisted environment variable. The ?= should
> therefore always being assigned as there wouldn't be a previous version of
> the variable.
> 
> I'd like to understand what value the variable is taking (which would stop the
> ?= applying).
> 
> Are you using BB_PRESERVE_ENV ?
> 

Hi,

I had another go at debugging. What happens is bitbake/lib/bb/cooker.py:166 reimports the environment after it has been cleaned (!). It is then recleaned at some time during the parsing of bitbake.conf, but after the conditional assignment of PSEUDO_LOCALSTATEDIR. I wasn't patient enough to find out when exactly.

Worse, I don't think bitbake.conf is guaranteed to be in the same spot in every layers.conf, so different layers will parse with different environments.

James

Re: [PATCH] bitbake.conf: Changed PSEUDO_LOCALSTATEDIR assignment to unconditional.

[James Limbouris]

> 
> Worse, I don't think bitbake.conf is guaranteed to be in the same spot in
> every layers.conf, so different layers will parse with different environments.
> 

Sorry, that last line is wrong. Bitbake.conf always parses last.

James