Re: [OE-core] [PATCH 1/2] bitbake.conf/sstate: Introduce DEFAULT_SHARED_UMASK to standarise shared area umask

[Richard Purdie <richard.purdie@linuxfoundation.org>]

On Mon, 2025-06-30 at 11:02 +0200, Rasmus Villemoes wrote:
> On Fri, Jun 27 2025, "Richard Purdie via lists.openembedded.org" <richard.purdie=linuxfoundation.org@lists.openembedded.org> wrote:
> 
> > Currently, the "shared" directory permissions of sstate are hardcoded. Since
> > multiple areas of the code reference this, separate it out to a variable to
> > allow the behaviour to be configurable. Initially this applies to SSTATE_DIR.
> > 
> > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> > ---
> >  meta/classes-global/sstate.bbclass | 12 +++++++-----
> >  meta/conf/bitbake.conf             |  2 ++
> >  2 files changed, 9 insertions(+), 5 deletions(-)
> > 
> > diff --git a/meta/classes-global/sstate.bbclass b/meta/classes-global/sstate.bbclass
> > index 2968cc4c2e7..7578aad24ea 100644
> > --- a/meta/classes-global/sstate.bbclass
> > +++ b/meta/classes-global/sstate.bbclass
> > @@ -745,7 +745,7 @@ def pstaging_fetch(sstatefetch, d):
> >      if bb.utils.to_boolean(d.getVar("SSTATE_VERIFY_SIG"), False):
> >          uris += ['file://{0}.sig;downloadfilename={0}.sig'.format(sstatefetch)]
> >  
> > -    with bb.utils.umask(0o002):
> > +    with bb.utils.umask(bb.utils.to_filemode(d.getVar("DEFAULT_SHARED_UMASK"))):
> >          bb.utils.mkdirhier(dldir)
> >  
> >          for srcuri in uris:
> > @@ -776,9 +776,10 @@ sstate_task_prefunc[dirs] = "${WORKDIR}"
> >  python sstate_task_postfunc () {
> >      shared_state = sstate_state_fromvars(d)
> >  
> > -    omask = os.umask(0o002)
> > -    if omask != 0o002:
> > -       bb.note("Using umask 0o002 (not %0o) for sstate packaging" % omask)
> > +    shared_umask = bb.utils.to_filemode(d.getVar("DEFAULT_SHARED_UMASK"))
> > +    omask = os.umask(shared_umask)
> > +    if omask != shared_umask:
> > +       bb.note("Using umask %0o (not %0o) for sstate packaging" % (shared_umask, omask))
> >      sstate_package(shared_state, d)
> >      os.umask(omask)
> >  
> > @@ -843,7 +844,8 @@ python sstate_create_and_sign_package () {
> >  
> >      # Create the required sstate directory if it is not present.
> >      if not sstate_pkg.parent.is_dir():
> > -        with bb.utils.umask(0o002):
> > +        shared_umask = bb.utils.to_filemode(d.getVar("DEFAULT_SHARED_UMASK"))
> > +        with bb.utils.umask(shared_umask):
> >              bb.utils.mkdirhier(str(sstate_pkg.parent))
> >  
> >      if sign_pkg:
> > diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
> > index a3300fc1727..22473bfe23a 100644
> > --- a/meta/conf/bitbake.conf
> > +++ b/meta/conf/bitbake.conf
> > @@ -944,6 +944,8 @@ TRANSLATED_TARGET_ARCH ??= "${@d.getVar('TARGET_ARCH').replace("_", "-")}"
> >  
> >  # Set a default umask to use for tasks for determinism
> >  BB_DEFAULT_UMASK ??= "022"
> > +# The umask to use for shared files (e.g. DL_DIR and SSTATE_DIR)
> > +DEFAULT_SHARED_UMASK ??= "002"
> 
> This is perhaps bikeshedding, but I think that naming is somewhat
> off. For BB_DEFAULT_UMASK, the "default" refers to this being used if
> the task doesn't have it's own [umask] flag.
> 
> For the new variable, yes, this setting is a default, but that's really
> the ??= part of the line. If someone wants to change the umask used for
> those 'shared' areas, they should just have to change
> "SHARED_UMASK". Otherwise we should also have DEFAULT_PARALLEL_MAKE etc.
> 
> I think I'd prefer a BB_ prefix, just to keep it a little namespaced,
> but I can see how this might not be a bitbake thing (unlike the variable
> that applies to tasks in general).

Naming is important and I agree, "DEFAULT" in there isn't right. We
can't really use BB_ as that is reserved for things bitbake itself
handles so I've updated a version to use OE_SHARED_UMASK.

> Should the 0775 instances in the test code be updated to be computed
> as 0777 minus [DEFAULT_]SHARED_UMASK or is it assumed that the tests
> are always run with default settings?

I wasn't sure what to do with that, we could change it as you suggest.
I guess we may end up needing to test multiple values.

Cheers,

Richard