[PATCH] isar-image-base: fix owner and group of /proc

[PATCH] isar-image-base: fix owner and group of /proc

[Christian Storm]

Signed-off-by: Christian Storm <christian.storm@siemens.com>
---
 meta-isar/recipes-core/images/isar-image-base.bb | 2 +-
 meta/recipes-devtools/buildchroot/buildchroot.bb | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/meta-isar/recipes-core/images/isar-image-base.bb b/meta-isar/recipes-core/images/isar-image-base.bb
index c2150b1..a6906c6 100644
--- a/meta-isar/recipes-core/images/isar-image-base.bb
+++ b/meta-isar/recipes-core/images/isar-image-base.bb
@@ -52,7 +52,7 @@ do_rootfs() {
         -e 's|##ISAR_DISTRO_SUITE##|${DEBDISTRONAME}|g' \
            "${WORKDIR}/multistrap.conf.in" > "${WORKDIR}/multistrap.conf"
 
-    [ ! -d ${IMAGE_ROOTFS}/proc ] && install -d -m 555 ${IMAGE_ROOTFS}/proc
+    [ ! -d ${IMAGE_ROOTFS}/proc ] && sudo install -d -o 0 -g 0 -m 555 ${IMAGE_ROOTFS}/proc
     sudo mount -t proc none ${IMAGE_ROOTFS}/proc
     _do_rootfs_cleanup() {
         ret=$?
diff --git a/meta/recipes-devtools/buildchroot/buildchroot.bb b/meta/recipes-devtools/buildchroot/buildchroot.bb
index 6a94733..f9bfae4 100644
--- a/meta/recipes-devtools/buildchroot/buildchroot.bb
+++ b/meta/recipes-devtools/buildchroot/buildchroot.bb
@@ -52,7 +52,7 @@ do_build() {
         -e 's|##DIR_HOOKS##|./'"$WORKDIR_REL"'/hooks_multistrap|g' \
            "${WORKDIR}/multistrap.conf.in" > "${WORKDIR}/multistrap.conf"
 
-    [ ! -d ${BUILDCHROOT_DIR}/proc ] && install -d -m 555 ${BUILDCHROOT_DIR}/proc
+    [ ! -d ${BUILDCHROOT_DIR}/proc ] && sudo install -d -o 0 -g 0 -m 555 ${BUILDCHROOT_DIR}/proc
     sudo mount -t proc none ${BUILDCHROOT_DIR}/proc
     _do_build_cleanup() {
         ret=$?
-- 
2.15.0

Re: [PATCH] isar-image-base: fix owner and group of /proc

[Alexander Smirnov]

Hi,

On 11/17/2017 12:54 PM, Christian Storm wrote:
> Signed-off-by: Christian Storm <christian.storm@siemens.com>
> ---
>   meta-isar/recipes-core/images/isar-image-base.bb | 2 +-
>   meta/recipes-devtools/buildchroot/buildchroot.bb | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/meta-isar/recipes-core/images/isar-image-base.bb b/meta-isar/recipes-core/images/isar-image-base.bb
> index c2150b1..a6906c6 100644
> --- a/meta-isar/recipes-core/images/isar-image-base.bb
> +++ b/meta-isar/recipes-core/images/isar-image-base.bb
> @@ -52,7 +52,7 @@ do_rootfs() {
>           -e 's|##ISAR_DISTRO_SUITE##|${DEBDISTRONAME}|g' \
>              "${WORKDIR}/multistrap.conf.in" > "${WORKDIR}/multistrap.conf"
>   
> -    [ ! -d ${IMAGE_ROOTFS}/proc ] && install -d -m 555 ${IMAGE_ROOTFS}/proc
> +    [ ! -d ${IMAGE_ROOTFS}/proc ] && sudo install -d -o 0 -g 0 -m 555 ${IMAGE_ROOTFS}/proc

What is the requirement for doing so? And what is the benefit?

Alex

>       sudo mount -t proc none ${IMAGE_ROOTFS}/proc
>       _do_rootfs_cleanup() {
>           ret=$?
> diff --git a/meta/recipes-devtools/buildchroot/buildchroot.bb b/meta/recipes-devtools/buildchroot/buildchroot.bb
> index 6a94733..f9bfae4 100644
> --- a/meta/recipes-devtools/buildchroot/buildchroot.bb
> +++ b/meta/recipes-devtools/buildchroot/buildchroot.bb
> @@ -52,7 +52,7 @@ do_build() {
>           -e 's|##DIR_HOOKS##|./'"$WORKDIR_REL"'/hooks_multistrap|g' \
>              "${WORKDIR}/multistrap.conf.in" > "${WORKDIR}/multistrap.conf"
>   
> -    [ ! -d ${BUILDCHROOT_DIR}/proc ] && install -d -m 555 ${BUILDCHROOT_DIR}/proc
> +    [ ! -d ${BUILDCHROOT_DIR}/proc ] && sudo install -d -o 0 -g 0 -m 555 ${BUILDCHROOT_DIR}/proc
>       sudo mount -t proc none ${BUILDCHROOT_DIR}/proc
>       _do_build_cleanup() {
>           ret=$?
> 

Re: [PATCH] isar-image-base: fix owner and group of /proc

[Christian Storm]

Hi Alex,

> On 11/17/2017 12:54 PM, Christian Storm wrote:
> > Signed-off-by: Christian Storm <christian.storm@siemens.com>
> > ---
> >   meta-isar/recipes-core/images/isar-image-base.bb | 2 +-
> >   meta/recipes-devtools/buildchroot/buildchroot.bb | 2 +-
> >   2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/meta-isar/recipes-core/images/isar-image-base.bb b/meta-isar/recipes-core/images/isar-image-base.bb
> > index c2150b1..a6906c6 100644
> > --- a/meta-isar/recipes-core/images/isar-image-base.bb
> > +++ b/meta-isar/recipes-core/images/isar-image-base.bb
> > @@ -52,7 +52,7 @@ do_rootfs() {
> >           -e 's|##ISAR_DISTRO_SUITE##|${DEBDISTRONAME}|g' \
> >              "${WORKDIR}/multistrap.conf.in" > "${WORKDIR}/multistrap.conf"
> >   
> > -    [ ! -d ${IMAGE_ROOTFS}/proc ] && install -d -m 555 ${IMAGE_ROOTFS}/proc
> > +    [ ! -d ${IMAGE_ROOTFS}/proc ] && sudo install -d -o 0 -g 0 -m 555 ${IMAGE_ROOTFS}/proc
> 
> What is the requirement for doing so? And what is the benefit?

In the resulting image, /proc was created with my build user's ownership
!= root as it was non-existent. To create the directory belonging to
root, sudo is required here...



Kind regards,
   Christian

-- 
Dr. Christian Storm
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Otto-Hahn-Ring 6, 81739 M�nchen, Germany

Re: [PATCH] isar-image-base: fix owner and group of /proc

[Alexander Smirnov]

Hi Christian,

On 11/17/2017 01:17 PM, Christian Storm wrote:
> Hi Alex,
> 
>> On 11/17/2017 12:54 PM, Christian Storm wrote:
>>> Signed-off-by: Christian Storm <christian.storm@siemens.com>
>>> ---
>>>    meta-isar/recipes-core/images/isar-image-base.bb | 2 +-
>>>    meta/recipes-devtools/buildchroot/buildchroot.bb | 2 +-
>>>    2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/meta-isar/recipes-core/images/isar-image-base.bb b/meta-isar/recipes-core/images/isar-image-base.bb
>>> index c2150b1..a6906c6 100644
>>> --- a/meta-isar/recipes-core/images/isar-image-base.bb
>>> +++ b/meta-isar/recipes-core/images/isar-image-base.bb
>>> @@ -52,7 +52,7 @@ do_rootfs() {
>>>            -e 's|##ISAR_DISTRO_SUITE##|${DEBDISTRONAME}|g' \
>>>               "${WORKDIR}/multistrap.conf.in" > "${WORKDIR}/multistrap.conf"
>>>    
>>> -    [ ! -d ${IMAGE_ROOTFS}/proc ] && install -d -m 555 ${IMAGE_ROOTFS}/proc
>>> +    [ ! -d ${IMAGE_ROOTFS}/proc ] && sudo install -d -o 0 -g 0 -m 555 ${IMAGE_ROOTFS}/proc
>>
>> What is the requirement for doing so? And what is the benefit?
> 
> In the resulting image, /proc was created with my build user's ownership
> != root as it was non-existent. To create the directory belonging to
> root, sudo is required here...
> 

Thanks, got it! But I don't think it's also required for buildchroot.

Alex

Re: [PATCH] isar-image-base: fix owner and group of /proc

[Christian Storm]

> >>> Signed-off-by: Christian Storm <christian.storm@siemens.com>
> >>> ---
> >>>    meta-isar/recipes-core/images/isar-image-base.bb | 2 +-
> >>>    meta/recipes-devtools/buildchroot/buildchroot.bb | 2 +-
> >>>    2 files changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/meta-isar/recipes-core/images/isar-image-base.bb b/meta-isar/recipes-core/images/isar-image-base.bb
> >>> index c2150b1..a6906c6 100644
> >>> --- a/meta-isar/recipes-core/images/isar-image-base.bb
> >>> +++ b/meta-isar/recipes-core/images/isar-image-base.bb
> >>> @@ -52,7 +52,7 @@ do_rootfs() {
> >>>            -e 's|##ISAR_DISTRO_SUITE##|${DEBDISTRONAME}|g' \
> >>>               "${WORKDIR}/multistrap.conf.in" > "${WORKDIR}/multistrap.conf"
> >>>    
> >>> -    [ ! -d ${IMAGE_ROOTFS}/proc ] && install -d -m 555 ${IMAGE_ROOTFS}/proc
> >>> +    [ ! -d ${IMAGE_ROOTFS}/proc ] && sudo install -d -o 0 -g 0 -m 555 ${IMAGE_ROOTFS}/proc
> >>
> >> What is the requirement for doing so? And what is the benefit?
> > 
> > In the resulting image, /proc was created with my build user's ownership
> > != root as it was non-existent. To create the directory belonging to
> > root, sudo is required here...
> > 
> 
> Thanks, got it! But I don't think it's also required for buildchroot.

Nope, not strictly required but for cosmetics and symmetry reasons I put
it in there as well. Doesn't do harm either..


Kind regards,
   Christian

-- 
Dr. Christian Storm
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Otto-Hahn-Ring 6, 81739 M�nchen, Germany

Re: [PATCH] isar-image-base: fix owner and group of /proc

[Alexander Smirnov]

Hi,

On 11/17/2017 07:34 PM, Christian Storm wrote:
>>>>> Signed-off-by: Christian Storm <christian.storm@siemens.com>
>>>>> ---
>>>>>     meta-isar/recipes-core/images/isar-image-base.bb | 2 +-
>>>>>     meta/recipes-devtools/buildchroot/buildchroot.bb | 2 +-
>>>>>     2 files changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/meta-isar/recipes-core/images/isar-image-base.bb b/meta-isar/recipes-core/images/isar-image-base.bb
>>>>> index c2150b1..a6906c6 100644
>>>>> --- a/meta-isar/recipes-core/images/isar-image-base.bb
>>>>> +++ b/meta-isar/recipes-core/images/isar-image-base.bb
>>>>> @@ -52,7 +52,7 @@ do_rootfs() {
>>>>>             -e 's|##ISAR_DISTRO_SUITE##|${DEBDISTRONAME}|g' \
>>>>>                "${WORKDIR}/multistrap.conf.in" > "${WORKDIR}/multistrap.conf"
>>>>>     
>>>>> -    [ ! -d ${IMAGE_ROOTFS}/proc ] && install -d -m 555 ${IMAGE_ROOTFS}/proc
>>>>> +    [ ! -d ${IMAGE_ROOTFS}/proc ] && sudo install -d -o 0 -g 0 -m 555 ${IMAGE_ROOTFS}/proc
>>>>
>>>> What is the requirement for doing so? And what is the benefit?
>>>
>>> In the resulting image, /proc was created with my build user's ownership
>>> != root as it was non-existent. To create the directory belonging to
>>> root, sudo is required here...
>>>
>>
>> Thanks, got it! But I don't think it's also required for buildchroot.
> 
> Nope, not strictly required but for cosmetics and symmetry reasons I put
> it in there as well. Doesn't do harm either..
> 

So I'm going to apply the first part of the patch (related to image) to 
avoid adding new 'sudo' without real usecase.

Also in context of dropping 'sudo' from Isar, image generation and 
buildchroot generation are not so symmetric. buildchroot has less strict 
requirements (like out of GID/UID support) what makes it possible to use 
PRoot to completely drop 'sudo' around buildchroot operations. So 
keeping image and buildchroot generation processes synchronized could 
bring unnecessary difficulties.

Alex

Re: [PATCH] isar-image-base: fix owner and group of /proc

[Christian Storm]

> >>>>> Signed-off-by: Christian Storm <christian.storm@siemens.com>
> >>>>> ---
> >>>>>     meta-isar/recipes-core/images/isar-image-base.bb | 2 +-
> >>>>>     meta/recipes-devtools/buildchroot/buildchroot.bb | 2 +-
> >>>>>     2 files changed, 2 insertions(+), 2 deletions(-)
> >>>>>
> >>>>> diff --git a/meta-isar/recipes-core/images/isar-image-base.bb b/meta-isar/recipes-core/images/isar-image-base.bb
> >>>>> index c2150b1..a6906c6 100644
> >>>>> --- a/meta-isar/recipes-core/images/isar-image-base.bb
> >>>>> +++ b/meta-isar/recipes-core/images/isar-image-base.bb
> >>>>> @@ -52,7 +52,7 @@ do_rootfs() {
> >>>>>             -e 's|##ISAR_DISTRO_SUITE##|${DEBDISTRONAME}|g' \
> >>>>>                "${WORKDIR}/multistrap.conf.in" > "${WORKDIR}/multistrap.conf"
> >>>>>     
> >>>>> -    [ ! -d ${IMAGE_ROOTFS}/proc ] && install -d -m 555 ${IMAGE_ROOTFS}/proc
> >>>>> +    [ ! -d ${IMAGE_ROOTFS}/proc ] && sudo install -d -o 0 -g 0 -m 555 ${IMAGE_ROOTFS}/proc
> >>>>
> >>>> What is the requirement for doing so? And what is the benefit?
> >>>
> >>> In the resulting image, /proc was created with my build user's ownership
> >>> != root as it was non-existent. To create the directory belonging to
> >>> root, sudo is required here...
> >>>
> >>
> >> Thanks, got it! But I don't think it's also required for buildchroot.
> > 
> > Nope, not strictly required but for cosmetics and symmetry reasons I put
> > it in there as well. Doesn't do harm either..
> > 
> 
> So I'm going to apply the first part of the patch (related to image) to 
> avoid adding new 'sudo' without real usecase.

Fine with me.


> Also in context of dropping 'sudo' from Isar, image generation and 
> buildchroot generation are not so symmetric. buildchroot has less strict 
> requirements (like out of GID/UID support) what makes it possible to use 
> PRoot to completely drop 'sudo' around buildchroot operations. So 
> keeping image and buildchroot generation processes synchronized could 
> bring unnecessary difficulties.

Well, currently they are quite similar in what they do. So, maybe the
other way round would also give some benefits, namely less duplication
in terms of code and logics.
Having just the buildchroot sudo-less while the image needs it doesn't
give you any benefit in my opinion. Everything has to work sudo-less or
you still need to rely on it, and be it only eventually for image
generation. In this case, at least for me, some more sprinkled sudos
cause no harm as I would have to run Isar in a VM or some other safe
build environment anyway. So, I'd rather have a more concise code base
without duplication in logics and code. But of course, your mileage may
vary on this...



Kind regards,
   Christian

-- 
Dr. Christian Storm
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Otto-Hahn-Ring 6, 81739 M�nchen, Germany

Re: [PATCH] isar-image-base: fix owner and group of /proc

[Alexander Smirnov]

>>>>>>>      meta-isar/recipes-core/images/isar-image-base.bb | 2 +-
>>>>>>>      meta/recipes-devtools/buildchroot/buildchroot.bb | 2 +-
>>>>>>>      2 files changed, 2 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/meta-isar/recipes-core/images/isar-image-base.bb b/meta-isar/recipes-core/images/isar-image-base.bb
>>>>>>> index c2150b1..a6906c6 100644
>>>>>>> --- a/meta-isar/recipes-core/images/isar-image-base.bb
>>>>>>> +++ b/meta-isar/recipes-core/images/isar-image-base.bb
>>>>>>> @@ -52,7 +52,7 @@ do_rootfs() {
>>>>>>>              -e 's|##ISAR_DISTRO_SUITE##|${DEBDISTRONAME}|g' \
>>>>>>>                 "${WORKDIR}/multistrap.conf.in" > "${WORKDIR}/multistrap.conf"
>>>>>>>      
>>>>>>> -    [ ! -d ${IMAGE_ROOTFS}/proc ] && install -d -m 555 ${IMAGE_ROOTFS}/proc
>>>>>>> +    [ ! -d ${IMAGE_ROOTFS}/proc ] && sudo install -d -o 0 -g 0 -m 555 ${IMAGE_ROOTFS}/proc
>>>>>>
>>>>>> What is the requirement for doing so? And what is the benefit?
>>>>>
>>>>> In the resulting image, /proc was created with my build user's ownership
>>>>> != root as it was non-existent. To create the directory belonging to
>>>>> root, sudo is required here...
>>>>>
>>>>
>>>> Thanks, got it! But I don't think it's also required for buildchroot.
>>>
>>> Nope, not strictly required but for cosmetics and symmetry reasons I put
>>> it in there as well. Doesn't do harm either..
>>>
>>
>> So I'm going to apply the first part of the patch (related to image) to
>> avoid adding new 'sudo' without real usecase.
> 
> Fine with me.
> 

Applied to next, thanks!