Re: [RFC PATCH 0/3] Shared state for all !

[Joshua Lock <josh@linux.intel.com>]

On 09/05/12 22:16, Khem Raj wrote:
> On Wed, May 9, 2012 at 8:14 PM, Joshua Lock<josh@linux.intel.com>  wrote:
>> On 09/05/12 20:10, Joshua Lock wrote:
>>>
>>> On 09/05/12 19:32, Joshua Lock wrote:
>>>>
>>>> On 09/05/12 19:15, Chris Larson wrote:
>>>>>
>>>>> On Wed, May 9, 2012 at 6:05 PM, Joshua Lock<josh@linux.intel.com>  wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 09/05/12 17:50, Chris Larson wrote:
>>>>>>>
>>>>>>>
>>>>>>> On Wed, May 9, 2012 at 5:22 PM, Joshua Lock<josh@linux.intel.com>
>>>>>>> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> 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.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> 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.
>>>>>
>>>>>
>>>>> 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
>>>>>
>>>>> 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.
>>
>>
>> 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.

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

Joshua
-- 
Joshua Lock
         Yocto Project
         Intel Open Source Technology Centre