Openembedded Core Discussions
 help / color / mirror / Atom feed
* [PATCH 1/2] bitbake.conf/sstate: Introduce DEFAULT_SHARED_UMASK to standarise shared area umask
@ 2025-06-27 21:24 Richard Purdie
  2025-06-27 21:24 ` [PATCH 2/2] base: Use DEFAULT_SHARED_UMASK for do_fetch Richard Purdie
  2025-06-30  9:02 ` [OE-core] [PATCH 1/2] bitbake.conf/sstate: Introduce DEFAULT_SHARED_UMASK to standarise shared area umask Rasmus Villemoes
  0 siblings, 2 replies; 6+ messages in thread
From: Richard Purdie @ 2025-06-27 21:24 UTC (permalink / raw)
  To: openembedded-core

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] base: Use DEFAULT_SHARED_UMASK for do_fetch
  2025-06-27 21:24 [PATCH 1/2] bitbake.conf/sstate: Introduce DEFAULT_SHARED_UMASK to standarise shared area umask Richard Purdie
@ 2025-06-27 21:24 ` Richard Purdie
  2025-06-30  9:18   ` [OE-core] " Rasmus Villemoes
  2025-06-30  9:02 ` [OE-core] [PATCH 1/2] bitbake.conf/sstate: Introduce DEFAULT_SHARED_UMASK to standarise shared area umask Rasmus Villemoes
  1 sibling, 1 reply; 6+ messages in thread
From: Richard Purdie @ 2025-06-27 21:24 UTC (permalink / raw)
  To: openembedded-core

The intent has always been to share DL_DIR, so set the umask accordingly
to the new DEFAULT_SHARED_UMASK variable and match expectations.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 meta/classes-global/base.bbclass | 1 +
 1 file changed, 1 insertion(+)

diff --git a/meta/classes-global/base.bbclass b/meta/classes-global/base.bbclass
index b86f50e2839..77bdbd03dcb 100644
--- a/meta/classes-global/base.bbclass
+++ b/meta/classes-global/base.bbclass
@@ -154,6 +154,7 @@ do_fetch[file-checksums] = "${@bb.fetch.get_checksum_file_list(d)}"
 do_fetch[file-checksums] += " ${@get_lic_checksum_file_list(d)}"
 do_fetch[prefuncs] += "fetcher_hashes_dummyfunc"
 do_fetch[network] = "1"
+do_fetch[umask] = "${DEFAULT_SHARED_UMASK}"
 python base_do_fetch() {
 
     src_uri = (d.getVar('SRC_URI') or "").split()


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [OE-core] [PATCH 1/2] bitbake.conf/sstate: Introduce DEFAULT_SHARED_UMASK to standarise shared area umask
  2025-06-27 21:24 [PATCH 1/2] bitbake.conf/sstate: Introduce DEFAULT_SHARED_UMASK to standarise shared area umask Richard Purdie
  2025-06-27 21:24 ` [PATCH 2/2] base: Use DEFAULT_SHARED_UMASK for do_fetch Richard Purdie
@ 2025-06-30  9:02 ` Rasmus Villemoes
  2025-06-30 11:21   ` Richard Purdie
  1 sibling, 1 reply; 6+ messages in thread
From: Rasmus Villemoes @ 2025-06-30  9:02 UTC (permalink / raw)
  To: richard.purdie; +Cc: openembedded-core

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [OE-core] [PATCH 2/2] base: Use DEFAULT_SHARED_UMASK for do_fetch
  2025-06-27 21:24 ` [PATCH 2/2] base: Use DEFAULT_SHARED_UMASK for do_fetch Richard Purdie
@ 2025-06-30  9:18   ` Rasmus Villemoes
  2025-06-30 11:23     ` Richard Purdie
  0 siblings, 1 reply; 6+ messages in thread
From: Rasmus Villemoes @ 2025-06-30  9:18 UTC (permalink / raw)
  To: richard.purdie; +Cc: openembedded-core

On Fri, Jun 27 2025, "Richard Purdie via lists.openembedded.org" <richard.purdie=linuxfoundation.org@lists.openembedded.org> wrote:

> The intent has always been to share DL_DIR, so set the umask accordingly
> to the new DEFAULT_SHARED_UMASK variable and match expectations.
>
> Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> ---
>  meta/classes-global/base.bbclass | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/meta/classes-global/base.bbclass b/meta/classes-global/base.bbclass
> index b86f50e2839..77bdbd03dcb 100644
> --- a/meta/classes-global/base.bbclass
> +++ b/meta/classes-global/base.bbclass
> @@ -154,6 +154,7 @@ do_fetch[file-checksums] = "${@bb.fetch.get_checksum_file_list(d)}"
>  do_fetch[file-checksums] += " ${@get_lic_checksum_file_list(d)}"
>  do_fetch[prefuncs] += "fetcher_hashes_dummyfunc"
>  do_fetch[network] = "1"
> +do_fetch[umask] = "${DEFAULT_SHARED_UMASK}"

So this changes metadata hash of everything. Perhaps that's the right
thing to do, but it does mean that changing that shared umask variable
comes with a rather high cost.

I was looking at doing the var lookup inside the download method
instead to hide this dependency.

But, I suppose, both the old and new umask variable are really more
"named constants" than things that are meant to be tweaked at all, as
they (especially BB_DEFAULT_UMASK) have wide-ranging effects that are
not really easy to predict.

Rasmus


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [OE-core] [PATCH 1/2] bitbake.conf/sstate: Introduce DEFAULT_SHARED_UMASK to standarise shared area umask
  2025-06-30  9:02 ` [OE-core] [PATCH 1/2] bitbake.conf/sstate: Introduce DEFAULT_SHARED_UMASK to standarise shared area umask Rasmus Villemoes
@ 2025-06-30 11:21   ` Richard Purdie
  0 siblings, 0 replies; 6+ messages in thread
From: Richard Purdie @ 2025-06-30 11:21 UTC (permalink / raw)
  To: Rasmus Villemoes; +Cc: openembedded-core

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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [OE-core] [PATCH 2/2] base: Use DEFAULT_SHARED_UMASK for do_fetch
  2025-06-30  9:18   ` [OE-core] " Rasmus Villemoes
@ 2025-06-30 11:23     ` Richard Purdie
  0 siblings, 0 replies; 6+ messages in thread
From: Richard Purdie @ 2025-06-30 11:23 UTC (permalink / raw)
  To: Rasmus Villemoes; +Cc: openembedded-core

On Mon, 2025-06-30 at 11:18 +0200, Rasmus Villemoes wrote:
> On Fri, Jun 27 2025, "Richard Purdie via lists.openembedded.org"
> <richard.purdie=linuxfoundation.org@lists.openembedded.org> wrote:
> 
> > The intent has always been to share DL_DIR, so set the umask
> > accordingly
> > to the new DEFAULT_SHARED_UMASK variable and match expectations.
> > 
> > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> > ---
> >  meta/classes-global/base.bbclass | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/meta/classes-global/base.bbclass b/meta/classes-
> > global/base.bbclass
> > index b86f50e2839..77bdbd03dcb 100644
> > --- a/meta/classes-global/base.bbclass
> > +++ b/meta/classes-global/base.bbclass
> > @@ -154,6 +154,7 @@ do_fetch[file-checksums] =
> > "${@bb.fetch.get_checksum_file_list(d)}"
> >  do_fetch[file-checksums] += " ${@get_lic_checksum_file_list(d)}"
> >  do_fetch[prefuncs] += "fetcher_hashes_dummyfunc"
> >  do_fetch[network] = "1"
> > +do_fetch[umask] = "${DEFAULT_SHARED_UMASK}"
> 
> So this changes metadata hash of everything. Perhaps that's the right
> thing to do, but it does mean that changing that shared umask
> variable comes with a rather high cost.

Since this value doesn't change the output, other than the mode of
files in SSTATE_DIR and DL_DIR, which have their own ABI of sorts, I'd
be ok with excluding OE_SHARED_UMASK from task hashes.

> I was looking at doing the var lookup inside the download method
> instead to hide this dependency.
> 
> But, I suppose, both the old and new umask variable are really more
> "named constants" than things that are meant to be tweaked at all, as
> they (especially BB_DEFAULT_UMASK) have wide-ranging effects that are
> not really easy to predict.

BB_DEFAULT_UMASK isn't meant to be changed. OE_SHARED_UMASK, could be
by users for security policies locally so I'm ok with making that work.

Cheers,

Richard


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-06-30 11:23 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-27 21:24 [PATCH 1/2] bitbake.conf/sstate: Introduce DEFAULT_SHARED_UMASK to standarise shared area umask Richard Purdie
2025-06-27 21:24 ` [PATCH 2/2] base: Use DEFAULT_SHARED_UMASK for do_fetch Richard Purdie
2025-06-30  9:18   ` [OE-core] " Rasmus Villemoes
2025-06-30 11:23     ` Richard Purdie
2025-06-30  9:02 ` [OE-core] [PATCH 1/2] bitbake.conf/sstate: Introduce DEFAULT_SHARED_UMASK to standarise shared area umask Rasmus Villemoes
2025-06-30 11:21   ` Richard Purdie

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox