Skip to contentSkip to conversation

Shared state for all !

RFC PATCH In review Joshua Lock 6 participants 20 messages May 10, 2012

lore
Share & tools
lore
#0 lore raw

cover Shared state for all !

This is an RFC series, and for ease of review sent as against the Poky tree so that it can be reviewed as a whole.

In Yocto #2041[2] Mark reported an issue with reusing shared state as a different user on the same machine.

Since the whole purpose of shared state is that it be shared I decided to dig into this issue. I wanted to at least be able to use the shared-state cache of a different user without error, even if all of the objects aren't actually used (i.e. native, at least on the Edison branch I did most of the testing with).

This is an RFC mainly because it changes the permissions of created directories, sstate files and siginfo files from what they have traditionally been.

There is more of the rhyme an reason in the patch commit headers and comments but tl;dr bb.mkdirhier directories will be 0777 (rwxrwxrwx) with this patch, as will all of the contents of sstate-cache (siginfo and tgz) files.

This is actually what one would expect from reading the Python API docs for os.makedirs "The default mode is 0777 (octal)."[1] but not what actually happens on most modern Linux systems thanks to umask.

Please review the following changes for suitability for inclusion. If you have any objections or suggestions for improvement, please respond to the patches. If you agree with the changes, please provide your Acked-by.

Regards,

Joshua

1. http://docs.python.org/library/os.html#os.makedirs
2. https://bugzilla.yoctoproject.org/show_bug.cgi?id=2041

The following changes since commit 6841abd17268a1c1ca5e3634f0ae9e21426ca9c9:

Hob: change the workflow about click 'Run Image' to run directly after builded a qemu image (2012-05-09 21:56:47 +0100)

are available in the git repository at:
git://git.yoctoproject.org/poky-contrib josh/perms
http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=josh/perms

just the BitBake changes are at:
git://github.com/incandescant/bitbake.git josh/perms
https://github.com/incandescant/bitbake/commits/josh/perms

just the openembedded-core changes are at:
git://git.openembedded.org/openembedded-core-contrib josh/perms
http://git.openembedded.org/openembedded-core-contrib/log/?h=josh/perms

Joshua Lock (3):
lib/bb/utils.py: add optional mode parameter to bb.utils.mkdirhier() lib/bb/siggen.py: create permissive files and directories sstate.bbclass: ensure sstate files are easily shared

 bitbake/lib/bb/siggen.py    |    7 ++++++-
 bitbake/lib/bb/utils.py     |    7 +++++--
 meta/classes/sstate.bbclass |    1 +
 3 files changed, 12 insertions(+), 3 deletions(-)
Signature · 1 line
1.7.7.6
#1 lore raw

RFC PATCH 1/3 lib/bb/utils.py: add optional mode parameter to bb.utils.mkdirhier()

The optional parameter should be an octal file mode representing the desired permissions of the created directory. If mode isn;t specified use 0777. This is the default of os.makedirs() yet is not often what the created directory ends up with due to umask affecting the call.

The use of chmod() in this patch ensures that whatever octal is passed are the permissions the directory will have after creation. This is a behaviour change...

---
 bitbake/lib/bb/utils.py |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)
bitbake/lib/bb/utils.py +5 -2
index 7a73419..33206fe 100644
@@ -531,13 +531,16 @@ def prune_suffix(var, suffixes, d):
             return var.replace(suffix, "")
     return var
 
-def mkdirhier(directory):
+def mkdirhier(directory, mode=0777):
     """Create a directory like 'mkdir -p', but does not complain if
     directory already exists like os.makedirs
     """
 
     try:
-        os.makedirs(directory)
+        os.makedirs(directory, mode)
+        # We can't rely on makedirs having set the mode correctly as it is
+        # affected by umask
+        os.chmod(directory, mode)
     except OSError as e:
         if e.errno != errno.EEXIST:
             raise e
Signature · 1 line
1.7.7.6
#2 lore raw

Re: [RFC PATCH 1/3] lib/bb/utils.py: add optional mode parameter to bb.utils.mkdirhier()

Quoted 27 lines · Joshua Lock

The optional parameter should be an octal file mode representing the desired permissions of the created directory. If mode isn;t specified use 0777. This is the default of os.makedirs() yet is not often what the created directory ends up with due to umask affecting the call.

The use of chmod() in this patch ensures that whatever octal is passed are the permissions the directory will have after creation. This is a behaviour change...

---
 bitbake/lib/bb/utils.py |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)
bitbake/lib/bb/utils.py +1 -1
index 7a73419..33206fe 100644
@@ -531,13 +531,16 @@ def prune_suffix(var, suffixes, d):
             return var.replace(suffix, "")
     return var
 
-def mkdirhier(directory):
+def mkdirhier(directory, mode=0777):
     """Create a directory like 'mkdir -p', but does not complain if
     directory already exists like os.makedirs
     """

bitbake/lib/bb/utils.py def prune_suffix(var, suffixes, d): · line 546

Quoted 10 lines
bitbake/lib/bb/utils.py +4 -1
 
     try:
-        os.makedirs(directory)
+        os.makedirs(directory, mode)
+        # We can't rely on makedirs having set the mode correctly as it is
+        # affected by umask
+        os.chmod(directory, mode)
     except OSError as e:
         if e.errno != errno.EEXIST:
             raise e

I'm afraid there are a few holes here. mkdirhier can create parents and your chmod doesn't change these. I also agree with the comments, I think this needs to remain unchanged behaviour wise, respecting the umask and only set a mode if its specified.

Cheers,

Richard

#3 lore raw

RFC PATCH 2/3 lib/bb/siggen.py: create permissive files and directories

In reply to Joshua Lock

Create signature files, and the directories which contain them, with rwx for everyone so that they are easily shared with different users of the same machine.

---
 bitbake/lib/bb/siggen.py |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)
bitbake/lib/bb/siggen.py +6 -1
index 8c79b17..bd6d59b 100644
@@ -201,7 +201,12 @@ class SignatureGeneratorBasic(SignatureGenerator):
             for dep in data['runtaskdeps']:
                 data['runtaskhashes'][dep] = self.taskhash[dep]
 
-        p = pickle.Pickler(file(sigfile, "wb"), -1)
+        # Create file with permissive (0777) read/write for easier sharing
+        f = os.fdopen(os.open(sigfile, os.O_RDWR|os.O_CREAT), "wb")
+        # os.open() and os.fdopen() are affected by the users umask so brute force
+        # the permissions with a call to chmod
+        os.chmod(sigfile, 0777)
+        p = pickle.Pickler(f, -1)
         p.dump(data)
 
     def dump_sigs(self, dataCache):
Signature · 1 line
1.7.7.6
#4 lore raw

Re: [RFC PATCH 2/3] lib/bb/siggen.py: create permissive files and directories

Quoted 15 lines · Joshua Lock

Create signature files, and the directories which contain them, with rwx for everyone so that they are easily shared with different users of the same machine.

---
 bitbake/lib/bb/siggen.py |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)
bitbake/lib/bb/siggen.py +0 -0
index 8c79b17..bd6d59b 100644
@@ -201,7 +201,12 @@ class SignatureGeneratorBasic(SignatureGenerator):
             for dep in data['runtaskdeps']:

bitbake/lib/bb/siggen.py class SignatureGeneratorBasic(SignatureGenerator): · line 210

Quoted 10 lines
bitbake/lib/bb/siggen.py +6 -1
                 data['runtaskhashes'][dep] = self.taskhash[dep]
 
-        p = pickle.Pickler(file(sigfile, "wb"), -1)
+        # Create file with permissive (0777) read/write for easier sharing
+        f = os.fdopen(os.open(sigfile, os.O_RDWR|os.O_CREAT), "wb")
+        # os.open() and os.fdopen() are affected by the users umask so brute force
+        # the permissions with a call to chmod
+        os.chmod(sigfile, 0777)
+        p = pickle.Pickler(f, -1)
         p.dump(data)

Why not just run the chmod after the original pickle code? Its not as if you avoid a race this way :/.

Cheers,

Richard

#5 lore raw

Re: [RFC PATCH 2/3] lib/bb/siggen.py: create permissive files and directories

Quoted 18 lines · Richard Purdie
Quoted 17 lines · Joshua Lock

Create signature files, and the directories which contain them, with rwx for everyone so that they are easily shared with different users of the same machine.

---
  bitbake/lib/bb/siggen.py |    7 ++++++-
  1 files changed, 6 insertions(+), 1 deletions(-)
bitbake/lib/bb/siggen.py +0 -0
index 8c79b17..bd6d59b 100644
@@ -201,7 +201,12 @@ class SignatureGeneratorBasic(SignatureGenerator):
              for dep in data['runtaskdeps']:
                  data['runtaskhashes'][dep] = self.taskhash[dep]
 
Quoted 11 lines

bitbake/lib/bb/siggen.py class SignatureGeneratorBasic(SignatureGenerator): · line 210

Quoted 8 lines
bitbake/lib/bb/siggen.py +6 -1
-        p = pickle.Pickler(file(sigfile, "wb"), -1)
+        # Create file with permissive (0777) read/write for easier sharing
+        f = os.fdopen(os.open(sigfile, os.O_RDWR|os.O_CREAT), "wb")
+        # os.open() and os.fdopen() are affected by the users umask so brute force
+        # the permissions with a call to chmod
+        os.chmod(sigfile, 0777)
+        p = pickle.Pickler(f, -1)
          p.dump(data)

Why not just run the chmod after the original pickle code? Its not as if you avoid a race this way :/.

Sure, that'd work as well.

Though from the general feedback it feels like this series is moot and all that we really want is a sanity check.

Cheers,
Joshua

Signature · 3 lines
Joshua Lock
        Yocto Project
        Intel Open Source Technology Centre
#6 lore raw

RFC PATCH 3/3 sstate.bbclass: ensure sstate files are easily shared

In reply to Joshua Lock

In order to make sstate cache's more easily shared ensure any user of the system has rwx permission by calling chown on sstate files after they're created.

---
 meta/classes/sstate.bbclass |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
meta/classes/sstate.bbclass +1 -0
index a8c98e5..6707ecf 100644
@@ -454,6 +454,7 @@ sstate_create_package () {
 	else
 		tar -cz --file=$TFILE --files-from=/dev/null
 	fi
+	chmod 0777 $TFILE
 	mv $TFILE ${SSTATE_PKG}
 
 	cd ${WORKDIR}
Signature · 1 line
1.7.7.6
#7 lore raw

Re: [RFC PATCH 3/3] sstate.bbclass: ensure sstate files are easily shared

Quoted 14 lines · Joshua Lock

In order to make sstate cache's more easily shared ensure any user of the system has rwx permission by calling chown on sstate files after they're created.

---
  meta/classes/sstate.bbclass |    1 +
  1 files changed, 1 insertions(+), 0 deletions(-)
meta/classes/sstate.bbclass +0 -0
index a8c98e5..6707ecf 100644

meta/classes/sstate.bbclass sstate_create_package () { · line 460

Quoted 7 lines
meta/classes/sstate.bbclass +1 -0
@@ -454,6 +454,7 @@ sstate_create_package () {
  	else
  		tar -cz --file=$TFILE --files-from=/dev/null
  	fi
+	chmod 0777 $TFILE
  	mv $TFILE ${SSTATE_PKG}
 
  	cd ${WORKDIR}

Why execute permission, and should it not be restricted to 664 for group level write access? Why would multiple users be writing to the same sstate file anyways once it's there it could be read-only since a change will trigger a new sstate file, not a re-write of the existing one.

Sau!

#8 lore raw

Re: [RFC PATCH 3/3] sstate.bbclass: ensure sstate files are easily shared

Quoted 21 lines · Saul Wold
Quoted 20 lines · Joshua Lock

In order to make sstate cache's more easily shared ensure any user of the system has rwx permission by calling chown on sstate files after they're created.

---
meta/classes/sstate.bbclass | 1 +

1 files changed, 1 insertions(+), 0 deletions(-)
meta/classes/sstate.bbclass +0 -0
index a8c98e5..6707ecf 100644
@@ -454,6 +454,7 @@ sstate_create_package () {

else
tar -cz --file=$TFILE --files-from=/dev/null
fi
+ chmod 0777 $TFILE
mv $TFILE ${SSTATE_PKG}

Quoted 5 lines
Quoted 1 line

cd ${WORKDIR}

Why execute permission, and should it not be restricted to 664 for group level write access? Why would multiple users be writing to the same sstate file anyways once it's there it could be read-only since a change will trigger a new sstate file, not a re-write of the existing one.

For permissions I figured that whichever I went with someone would have an alternative suggestion so I went with as indiscriminate as possible - that's the main reason this is an RFC.

When I was reproducing the bug and had read-only siginfo file things blew up, so I created them writeable, see [1].

Cheers,
Joshua

1. https://bugzilla.yoctoproject.org/show_bug.cgi?id=2041#c9

Signature · 3 lines
Joshua Lock
        Yocto Project
        Intel Open Source Technology Centre
#9 lore raw
In reply to Joshua Lock
Quoted 15 lines · Joshua Lock

In Yocto #2041[2]  Mark reported an issue with reusing shared state as a different user on the same machine.

Since the whole purpose of shared state is that it be shared I decided to dig into this issue. I wanted to at least be able to use the shared-state cache of a different user without error, even if all of the objects aren't actually used (i.e. native, at least on the Edison branch I did most of the testing with).

This is an RFC mainly because it changes the permissions of created directories, sstate files and siginfo files from what they have traditionally been.

There is more of the rhyme an reason in the patch commit headers and comments but tl;dr bb.mkdirhier directories will be 0777 (rwxrwxrwx) with this patch, as will all of the contents of sstate-cache (siginfo and tgz) files.

Quoted 7 lines

This is actually what one would expect from reading the Python API docs for os.makedirs "The default mode is 0777 (octal)."[1] but not what actually happens on most modern Linux systems thanks to umask.

Please review the following changes for suitability for inclusion. If you have any objections or suggestions for improvement, please respond to the patches. If you agree with the changes, please provide your Acked-by.

777 seems questionable to me, personally. Generally collaboration happens amongst folks within a group, and chmod g+s makes that easier. I'd expect 775 to be a more sane value, myself.

Signature · 1 line
Christopher Larson
#10 lore raw
Quoted 20 lines · Chris Larson
Quoted 19 lines · Joshua Lock

In Yocto #2041[2] Mark reported an issue with reusing shared state as a different user on the same machine.

Since the whole purpose of shared state is that it be shared I decided to dig into this issue. I wanted to at least be able to use the shared-state cache of a different user without error, even if all of the objects aren't actually used (i.e. native, at least on the Edison branch I did most of the testing with).

This is an RFC mainly because it changes the permissions of created directories, sstate files and siginfo files from what they have traditionally been.

There is more of the rhyme an reason in the patch commit headers and comments but tl;dr bb.mkdirhier directories will be 0777 (rwxrwxrwx) with this patch, as will all of the contents of sstate-cache (siginfo and tgz) files.

This is actually what one would expect from reading the Python API docs for os.makedirs "The default mode is 0777 (octal)."[1] but not what actually happens on most modern Linux systems thanks to umask.

Quoted 7 lines
Quoted 3 lines

Please review the following changes for suitability for inclusion. If you have any objections or suggestions for improvement, please respond to the patches. If you agree with the changes, please provide your Acked-by.

777 seems questionable to me, personally. Generally collaboration happens amongst folks within a group, and chmod g+s makes that easier. I'd expect 775 to be a more sane value, myself.

Do you mean for bb.mkdirhier calls, the tgz files, the siginfo files or everything?

I went with 777 for mkdirhier as that's the default of os.makedirs before umask is involved. I would likely have picked rw-rw-r-- (664) if I weren't trying to request comments.

Cheers,
Joshua

Signature · 3 lines
Joshua Lock
        Yocto Project
        Intel Open Source Technology Centre
#11 lore raw
Quoted 46 lines · Joshua Lock
Quoted 38 lines · Chris Larson
Quoted 29 lines · Joshua Lock

In Yocto #2041[2]  Mark reported an issue with reusing shared state as a different user on the same machine.

Since the whole purpose of shared state is that it be shared I decided to dig
into this issue. I wanted to at least be able to use the shared-state cache of
a different user without error, even if all of the objects aren't actually used
(i.e. native, at least on the Edison branch I did most of the testing with).

This is an RFC mainly because it changes the permissions of created directories,
sstate files and siginfo files from what they have traditionally been.

There is more of the rhyme an reason in the patch commit headers and comments
but tl;dr bb.mkdirhier directories will be 0777 (rwxrwxrwx) with this patch, as
will all of the contents of sstate-cache (siginfo and tgz) files.

This is actually what one would expect from reading the Python API docs for
os.makedirs "The default mode is 0777 (octal)."[1] but not what actually happens
on most modern Linux systems thanks to umask.

Quoted 5 lines

Please review the following changes for suitability for inclusion. If you have
any objections or suggestions for improvement, please respond to the patches. If
you agree with the changes, please provide your Acked-by.

Quoted 3 lines

777 seems questionable to me, personally. Generally collaboration happens amongst folks within a group, and chmod g+s makes that easier. I'd expect 775 to be a more sane value, myself.

Quoted 6 lines

Do you mean for bb.mkdirhier calls, the tgz files, the siginfo files or everything?

I went with 777 for mkdirhier as that's the default of os.makedirs before umask is involved. I would likely have picked rw-rw-r-- (664) if I weren't trying to request comments.

Gotcha.

I'm concerned about the behavior change and potential implications of changing the default behavior of mkdirhier. I'm inclined to say that when you don't pass mode, let it use the current behavior of obeying the umask. If we're not going to do that, and want to change the default behavior, then I think 777 is the wrong/questionable default. Beyond that, 777 is certainly the wrong mode to be using for the shared state package in sstate.bbclass.

Signature · 1 line
Christopher Larson
#12 lore raw
Quoted 54 lines · Chris Larson
Quoted 46 lines · Joshua Lock
Quoted 38 lines · Chris Larson
Quoted 29 lines · Joshua Lock

In Yocto #2041[2] Mark reported an issue with reusing shared state as a different user on the same machine.

Since the whole purpose of shared state is that it be shared I decided to dig
into this issue. I wanted to at least be able to use the shared-state cache of
a different user without error, even if all of the objects aren't actually used
(i.e. native, at least on the Edison branch I did most of the testing with).

This is an RFC mainly because it changes the permissions of created directories,
sstate files and siginfo files from what they have traditionally been.

There is more of the rhyme an reason in the patch commit headers and comments
but tl;dr bb.mkdirhier directories will be 0777 (rwxrwxrwx) with this patch, as
will all of the contents of sstate-cache (siginfo and tgz) files.

This is actually what one would expect from reading the Python API docs for
os.makedirs "The default mode is 0777 (octal)."[1] but not what actually happens
on most modern Linux systems thanks to umask.

Quoted 5 lines

Please review the following changes for suitability for inclusion. If you have
any objections or suggestions for improvement, please respond to the patches. If
you agree with the changes, please provide your Acked-by.

Quoted 3 lines

777 seems questionable to me, personally. Generally collaboration happens amongst folks within a group, and chmod g+s makes that easier. I'd expect 775 to be a more sane value, myself.

Quoted 6 lines

Do you mean for bb.mkdirhier calls, the tgz files, the siginfo files or everything?

I went with 777 for mkdirhier as that's the default of os.makedirs before umask is involved. I would likely have picked rw-rw-r-- (664) if I weren't trying to request comments.

Quoted 6 lines

Gotcha.

I'm concerned about the behavior change and potential implications of changing the default behavior of mkdirhier. I'm inclined to say that when you don't pass mode, let it use the current behavior of obeying the umask.

An earlier version of the series did this and I'm happy to add that behaviour back in.

If we're not going to do that, and want to change the

Quoted 3 lines

default behavior, then I think 777 is the wrong/questionable default. Beyond that, 777 is certainly the wrong mode to be using for the shared state package in sstate.bbclass.

Do you have a strong preference on 664 vs. 775 ?

Thanks for the comments and feedback,

Joshua

Signature · 3 lines
Joshua Lock
        Yocto Project
        Intel Open Source Technology Centre
#13 lore raw
Quoted 73 lines · Joshua Lock
Quoted 62 lines · Chris Larson
Quoted 52 lines · Joshua Lock
Quoted 44 lines · Chris Larson
Quoted 33 lines · Joshua Lock

In Yocto #2041[2] Mark reported an issue with reusing shared state as a
different user on the same machine.

Since the whole purpose of shared state is that it be shared I decided to
dig
into this issue. I wanted to at least be able to use the shared-state cache of
a different user without error, even if all of the objects aren't actually used
(i.e. native, at least on the Edison branch I did most of the testing with).

This is an RFC mainly because it changes the permissions of created directories,
sstate files and siginfo files from what they have traditionally been.

There is more of the rhyme an reason in the patch commit headers and comments
but tl;dr bb.mkdirhier directories will be 0777 (rwxrwxrwx) with this patch, as
will all of the contents of sstate-cache (siginfo and tgz) files.

This is actually what one would expect from reading the Python API docs
for
os.makedirs "The default mode is 0777 (octal)."[1] but not what actually
happens
on most modern Linux systems thanks to umask.

Quoted 6 lines

Please review the following changes for suitability for inclusion. If you
have
any objections or suggestions for improvement, please respond to the patches. If
you agree with the changes, please provide your Acked-by.

Quoted 3 lines

777 seems questionable to me, personally. Generally collaboration happens amongst folks within a group, and chmod g+s makes that easier. I'd expect 775 to be a more sane value, myself.

Quoted 8 lines

Do you mean for bb.mkdirhier calls, the tgz files, the siginfo files or everything?

I went with 777 for mkdirhier as that's the default of os.makedirs before
umask is involved. I would likely have picked rw-rw-r-- (664) if I weren't
trying to request comments.

Quoted 6 lines

Gotcha.

I'm concerned about the behavior change and potential implications of changing the default behavior of mkdirhier. I'm inclined to say that when you don't pass mode, let it use the current behavior of obeying the umask.

An earlier version of the series did this and I'm happy to add that behaviour back in.

Quoted 6 lines

If we're not going to do that, and want to change the

Quoted 3 lines

default behavior, then I think 777 is the wrong/questionable default. Beyond that, 777 is certainly the wrong mode to be using for the shared state package in sstate.bbclass.

Do you have a strong preference on 664 vs. 775 ?

I'm leaning towards 664 (rw-rw-r--) for the files and 775 (rwxrwxr-x) for directories, these are the defaults for file and directory creation on Ubuntu 12.04 and Fedora 16.

Cheers,
Joshua

Signature · 3 lines
Joshua Lock
        Yocto Project
        Intel Open Source Technology Centre
#14 lore raw
Quoted 80 lines · Joshua Lock
Quoted 79 lines · Joshua Lock
Quoted 63 lines · Chris Larson
Quoted 53 lines · Joshua Lock
Quoted 45 lines · Chris Larson
Quoted 34 lines · Joshua Lock

In Yocto #2041[2] Mark reported an issue with reusing shared state as a
different user on the same machine.

Since the whole purpose of shared state is that it be shared I decided to
dig
into this issue. I wanted to at least be able to use the shared-state cache of
a different user without error, even if all of the objects aren't actually used
(i.e. native, at least on the Edison branch I did most of the testing with).

This is an RFC mainly because it changes the permissions of created directories,
sstate files and siginfo files from what they have traditionally been.

There is more of the rhyme an reason in the patch commit headers and comments
but tl;dr bb.mkdirhier directories will be 0777 (rwxrwxrwx) with this patch, as
will all of the contents of sstate-cache (siginfo and tgz) files.

This is actually what one would expect from reading the Python API docs
for
os.makedirs "The default mode is 0777 (octal)."[1] but not what actually
happens
on most modern Linux systems thanks to umask.

Quoted 6 lines

Please review the following changes for suitability for inclusion. If you
have
any objections or suggestions for improvement, please respond to the patches. If
you agree with the changes, please provide your Acked-by.

Quoted 3 lines

777 seems questionable to me, personally. Generally collaboration happens amongst folks within a group, and chmod g+s makes that easier. I'd expect 775 to be a more sane value, myself.

Quoted 8 lines

Do you mean for bb.mkdirhier calls, the tgz files, the siginfo files or everything?

I went with 777 for mkdirhier as that's the default of os.makedirs before
umask is involved. I would likely have picked rw-rw-r-- (664) if I weren't
trying to request comments.

Quoted 6 lines

Gotcha.

I'm concerned about the behavior change and potential implications of changing the default behavior of mkdirhier. I'm inclined to say that when you don't pass mode, let it use the current behavior of obeying the umask.

An earlier version of the series did this and I'm happy to add that behaviour back in.

If we're not going to do that, and want to change the

Quoted 3 lines

default behavior, then I think 777 is the wrong/questionable default. Beyond that, 777 is certainly the wrong mode to be using for the shared state package in sstate.bbclass.

Quoted 5 lines
Quoted 1 line

Do you have a strong preference on 664 vs. 775 ?

I'm leaning towards 664 (rw-rw-r--) for the files and 775 (rwxrwxr-x) for directories, these are the defaults for file and directory creation on Ubuntu 12.04 and Fedora 16.

At which point, the obvious "solution" to the bug report is a sanity check whether the user can read and write to the sstate directory.

Cheers,
Joshua

Signature · 3 lines
Joshua Lock
        Yocto Project
        Intel Open Source Technology Centre
#15 lore raw
Quoted 96 lines · Joshua Lock
Quoted 95 lines · Joshua Lock
Quoted 91 lines · Joshua Lock
Quoted 70 lines · Chris Larson
Quoted 58 lines · Joshua Lock
Quoted 48 lines · Chris Larson
Quoted 35 lines · Joshua Lock

In Yocto #2041[2] Mark reported an issue with reusing shared state as a
different user on the same machine.

Since the whole purpose of shared state is that it be shared I decided to
dig
into this issue. I wanted to at least be able to use the shared-state cache of
a different user without error, even if all of the objects aren't actually used
(i.e. native, at least on the Edison branch I did most of the testing with).

This is an RFC mainly because it changes the permissions of created directories,
sstate files and siginfo files from what they have traditionally been.

There is more of the rhyme an reason in the patch commit headers and comments
but tl;dr bb.mkdirhier directories will be 0777 (rwxrwxrwx) with this patch, as
will all of the contents of sstate-cache (siginfo and tgz) files.

This is actually what one would expect from reading the Python API docs
for
os.makedirs "The default mode is 0777 (octal)."[1] but not what actually
happens
on most modern Linux systems thanks to umask.

Quoted 6 lines

Please review the following changes for suitability for inclusion. If you
have
any objections or suggestions for improvement, please respond to the patches. If
you agree with the changes, please provide your Acked-by.

Quoted 3 lines

777 seems questionable to me, personally. Generally collaboration happens amongst folks within a group, and chmod g+s makes that easier. I'd expect 775 to be a more sane value, myself.

Quoted 8 lines

Do you mean for bb.mkdirhier calls, the tgz files, the siginfo files or everything?

I went with 777 for mkdirhier as that's the default of os.makedirs before
umask is involved. I would likely have picked rw-rw-r-- (664) if I weren't
trying to request comments.

Quoted 6 lines

Gotcha.

I'm concerned about the behavior change and potential implications of changing the default behavior of mkdirhier. I'm inclined to say that when you don't pass mode, let it use the current behavior of obeying the umask.

An earlier version of the series did this and I'm happy to add that behaviour back in.

If we're not going to do that, and want to change the

Quoted 4 lines

default behavior, then I think 777 is the wrong/questionable default. Beyond that, 777 is certainly the wrong mode to be using for the shared state package in sstate.bbclass.

Do you have a strong preference on 664 vs. 775 ?

Quoted 7 lines
Quoted 3 lines

I'm leaning towards 664 (rw-rw-r--) for the files and 775 (rwxrwxr-x) for directories, these are the defaults for file and directory creation on Ubuntu 12.04 and Fedora 16.

At which point, the obvious "solution" to the bug report is a sanity check whether the user can read and write to the sstate directory.

may be its already taken care of but I will ask anyway, if shared state is writable for all
how is contention resolved when someone tries to update the shared state since he/she rebuilt the package for some reason. I think having a global shared cache to read from but not update it would be something interesting where shared state is updated by say an autobuilder and used by developers in their local builds.

On perm issue could it be made that if the file is readable by user then while unpacking it gets the umask of current user ?

-Khem

#16 lore raw
Quoted 105 lines · Khem Raj
Quoted 96 lines · Joshua Lock
Quoted 95 lines · Joshua Lock
Quoted 91 lines · Joshua Lock
Quoted 70 lines · Chris Larson
Quoted 58 lines · Joshua Lock
Quoted 48 lines · Chris Larson
Quoted 35 lines · Joshua Lock

In Yocto #2041[2] Mark reported an issue with reusing shared state as a
different user on the same machine.

Since the whole purpose of shared state is that it be shared I decided to
dig
into this issue. I wanted to at least be able to use the shared-state cache of
a different user without error, even if all of the objects aren't actually used
(i.e. native, at least on the Edison branch I did most of the testing with).

This is an RFC mainly because it changes the permissions of created directories,
sstate files and siginfo files from what they have traditionally been.

There is more of the rhyme an reason in the patch commit headers and comments
but tl;dr bb.mkdirhier directories will be 0777 (rwxrwxrwx) with this patch, as
will all of the contents of sstate-cache (siginfo and tgz) files.

This is actually what one would expect from reading the Python API docs
for
os.makedirs "The default mode is 0777 (octal)."[1] but not what actually
happens
on most modern Linux systems thanks to umask.

Quoted 6 lines

Please review the following changes for suitability for inclusion. If you
have
any objections or suggestions for improvement, please respond to the patches. If
you agree with the changes, please provide your Acked-by.

Quoted 3 lines

777 seems questionable to me, personally. Generally collaboration happens amongst folks within a group, and chmod g+s makes that easier. I'd expect 775 to be a more sane value, myself.

Quoted 8 lines

Do you mean for bb.mkdirhier calls, the tgz files, the siginfo files or everything?

I went with 777 for mkdirhier as that's the default of os.makedirs before
umask is involved. I would likely have picked rw-rw-r-- (664) if I weren't
trying to request comments.

Quoted 6 lines

Gotcha.

I'm concerned about the behavior change and potential implications of changing the default behavior of mkdirhier. I'm inclined to say that when you don't pass mode, let it use the current behavior of obeying the umask.

An earlier version of the series did this and I'm happy to add that behaviour back in.

If we're not going to do that, and want to change the

Quoted 4 lines

default behavior, then I think 777 is the wrong/questionable default. Beyond that, 777 is certainly the wrong mode to be using for the shared state package in sstate.bbclass.

Do you have a strong preference on 664 vs. 775 ?

Quoted 7 lines
Quoted 3 lines

I'm leaning towards 664 (rw-rw-r--) for the files and 775 (rwxrwxr-x) for directories, these are the defaults for file and directory creation on Ubuntu 12.04 and Fedora 16.

At which point, the obvious "solution" to the bug report is a sanity check whether the user can read and write to the sstate directory.

Quoted 7 lines

may be its already taken care of but I will ask anyway, if shared state is writable for all
how is contention resolved when someone tries to update the shared state since he/she rebuilt the package for some reason. I think having a global shared cache to read from but not update it would be something interesting where shared state is updated by say an autobuilder and used by developers in their local builds.

SSTATE_MIRRORS does this nicely and since it uses the fetcher code that can be a file:// URI or other.

Joshua

Signature · 3 lines
Joshua Lock
        Yocto Project
        Intel Open Source Technology Centre
#17 lore raw
Quoted 3 lines · Joshua Lock

SSTATE_MIRRORS does this nicely and since it uses the fetcher code that can be a file:// URI or other.

yes figured yesterday night when I was thinking about it loud

#18 lore raw
In reply to Joshua Lock

Op 10 mei 2012, om 02:22 heeft Joshua Lock het volgende geschreven:

Quoted 1 line

This is an RFC series, and for ease of review sent as against the Poky tree

It requires an extra step to apply and thus makes review harder for me since my motivation to test them drops below zero. Please send patches to bitbake and oe-core to this list and reserve poky patches for the poky list. As for ease of review by glancing over them: 3 emails are 3 emails.

#19 lore raw
Quoted 7 lines · Koen Kooi

Op 10 mei 2012, om 02:22 heeft Joshua Lock het volgende geschreven:

Quoted 1 line

This is an RFC series, and for ease of review sent as against the Poky tree

It requires an extra step to apply and thus makes review harder for me since my motivation to test them drops below zero. Please send patches to bitbake and oe-core to this list and reserve poky patches for the poky list. As for ease of review by glancing over them: 3 emails are 3 emails.

BitBake and OE-Core branches were included in the email, though I didn't explicitly point that out.

Joshua

Signature · 3 lines
Joshua Lock
        Yocto Project
        Intel Open Source Technology Centre