diff mbox series

[1/2] bitbake.conf/sstate: Introduce DEFAULT_SHARED_UMASK to standarise shared area umask

Message ID 20250627212423.3315152-1-richard.purdie@linuxfoundation.org
State New
Headers show
Series [1/2] bitbake.conf/sstate: Introduce DEFAULT_SHARED_UMASK to standarise shared area umask | expand

Commit Message

Richard Purdie June 27, 2025, 9:24 p.m. UTC
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(-)

Comments

Rasmus Villemoes June 30, 2025, 9:02 a.m. UTC | #1
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).

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?

Rasmus
Richard Purdie June 30, 2025, 11:21 a.m. UTC | #2
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
diff mbox series

Patch

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"
 
 # Complete output from bitbake
 BB_CONSOLELOG ?= "${LOG_DIR}/cooker/${MACHINE}/${DATETIME}.log"