From mboxrd@z Thu Jan 1 00:00:00 1970 X-GM-THRID: 6864480010557718528 Date: Sat, 5 Sep 2020 08:19:05 -0700 (PDT) From: "vijaikumar....@gmail.com" To: isar-users Message-Id: <4bd30048-e7bf-486a-b500-d188086b8c9cn@googlegroups.com> In-Reply-To: <20200905103806.37dc1ba9@md1za8fc.ad001.siemens.net> References: <20200902185624.15044-1-Vijaikumar_Kanagarajan@mentor.com> <20200902185624.15044-7-Vijaikumar_Kanagarajan@mentor.com> <20200905103806.37dc1ba9@md1za8fc.ad001.siemens.net> Subject: Re: [PATCH v2 06/10] wic_fakeroot: Handle standalone pseudo invocations MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_Part_1994_500397524.1599319145917" X-TUID: R+lQsZJzARid ------=_Part_1994_500397524.1599319145917 Content-Type: multipart/alternative; boundary="----=_Part_1995_2046633043.1599319145917" ------=_Part_1995_2046633043.1599319145917 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On Saturday, September 5, 2020 at 2:08:07 PM UTC+5:30 Henning Schild wrote: > On Thu, 3 Sep 2020 00:26:20 +0530 > Vijai Kumar K wrote: > > > When using --exclude-path option wic copies the rootfs to a new > > location and invokes pseudo as a standalone command to rebuild the > > database in the new rootfs. > > > > This is not applicable when using wic_fakeroot. Return 0 for such > > standalone invocations in wic_fakeroot. > > > > It also looks for files.db inside the pseudo directory and throws an > > exception if it is not found. Handle that too. > > > > Signed-off-by: Vijai Kumar K > > --- > > meta/classes/wic-img.bbclass | 1 + > > scripts/wic_fakeroot | 5 +++++ > > 2 files changed, 6 insertions(+) > > > > diff --git a/meta/classes/wic-img.bbclass > > b/meta/classes/wic-img.bbclass index a2c9627..b1a7259 100644 > > --- a/meta/classes/wic-img.bbclass > > +++ b/meta/classes/wic-img.bbclass > > @@ -144,6 +144,7 @@ EOSUDO > > export BUILDDIR=${BUILDDIR} > > export MTOOLS_SKIP_CHECK=1 > > mkdir -p ${IMAGE_ROOTFS}/../pseudo > > + touch ${IMAGE_ROOTFS}/../pseudo/files.db > > Where is this coming from? It is not mentioned in the commit message > and not used in the code. > This is to handle [2]. These kind of workarounds come because we use fakeroot. And our fakeroot was just to handle the fsck issue in stretch. That issue is still there in stretch package. The other approach is to drop the wic_fakeroot and these subsequent quirks handling and carry one patch on top of wic just for the fsck support in stretch. Anyway I proceeded with wic_fakeroot assuming that it might be useful when facing such package compatibility issues. But I see that has become an overhead. If only we can carry one patch on top of wic this all touch pseudo/files.db, startswith(-) quirks are not needed. [2]https://github.com/openembedded/openembedded-core/blob/404292b570a78895a1c7900eeb319e36e31dec20/scripts/lib/wic/plugins/source/rootfs.py#L130 > > > > # create the temp dir in the buildchroot to ensure uniqueness > > WICTMP=$(cd ${BUILDCHROOT_DIR}; mktemp -d -p tmp) > > diff --git a/scripts/wic_fakeroot b/scripts/wic_fakeroot > > index 88a03fa..16b011e 100755 > > --- a/scripts/wic_fakeroot > > +++ b/scripts/wic_fakeroot > > @@ -25,6 +25,11 @@ cmd = args[0] > > # rootfs/root ... > > assert os.geteuid() == 0, "wic_fakeroot must be run as root!" > > > > +# Check if we are calling the pseudo command itself. Return 1 > > +# for standalone pseudo operations. > > +if cmd.startswith('-'): > > + sys.exit(0) > > I find it hard to match the comment to the code i see. "-" means its > not a cmd but an arg to wic_fakeroot? And what about the 0 vs 1. > Yes. There is an instance[1] where FAKEROOT, in case of oe the pseudo, is called as a standalone command with options. We are checking whether the first option is an argument starting with '-' returning 0 on such calls. Good catch. I will fix the comment. [1] https://github.com/openembedded/openembedded-core/blob/404292b570a78895a1c7900eeb319e36e31dec20/scripts/lib/wic/plugins/source/rootfs.py#L133 > Henning > > > # e2fsck <= 1.43.5 returns 1 on non-errors (stretch and before > > affected) # treat 1 as safe ... the filesystem was successfully > > repaired and is OK if cmd.startswith('fsck.'): > > ------=_Part_1995_2046633043.1599319145917 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: quoted-printable

O= n Saturday, September 5, 2020 at 2:08:07 PM UTC+5:30 Henning Schild wrote:<= br>
On Thu, 3 Sep = 2020 00:26:20 +0530
Vijai Kumar K <= Vijaikumar_...@mentor.com> wrote:

> When using --exclude-path option wic copies the rootfs to a new
> location and invokes pseudo as a standalone command to rebuild the
> database in the new rootfs.
>=20
> This is not applicable when using wic_fakeroot. Return 0 for such
> standalone invocations in wic_fakeroot.
>=20
> It also looks for files.db inside the pseudo directory and throws = an
> exception if it is not found. Handle that too.
>=20
> Signed-off-by: Vijai Kumar K <Vijaikumar_...@mentor.com>
> ---
> meta/classes/wic-img.bbclass | 1 +
> scripts/wic_fakeroot | 5 +++++
> 2 files changed, 6 insertions(+)
>=20
> diff --git a/meta/classes/wic-img.bbclass
> b/meta/classes/wic-img.bbclass index a2c9627..b1a7259 100644
> --- a/meta/classes/wic-img.bbclass
> +++ b/meta/classes/wic-img.bbclass
> @@ -144,6 +144,7 @@ EOSUDO
> export BUILDDIR=3D${BUILDDIR}
> export MTOOLS_SKIP_CHECK=3D1
> mkdir -p ${IMAGE_ROOTFS}/../pseudo
> + touch ${IMAGE_ROOTFS}/../pseudo/files.db

Where is this coming from? It is not mentioned in the commit message
and not used in the code.

This is to handle [2].

<= /div>
These kind of workarounds come because we use fakeroot. And our f= akeroot was just
to handle the fsck issue in stretch. That issue = is still there in stretch package. The other approach
is to drop = the wic_fakeroot and these subsequent quirks handling and carry one patch o= n top of wic
just for the fsck support in stretch. Anyway I proce= eded with wic_fakeroot assuming that it might be
useful when faci= ng such package compatibility issues. But I see that has become an overhead= . If only
we can carry one patch on top of wic this all touch pse= udo/files.db, startswith(-) quirks are not needed. 

[2]https://github.com/openembedded/openembedded-core/blob/404292b570= a78895a1c7900eeb319e36e31dec20/scripts/lib/wic/plugins/source/rootfs.py#L13= 0 


> =20
> # create the temp dir in the buildchroot to ensure uniqueness
> WICTMP=3D$(cd ${BUILDCHROOT_DIR}; mktemp -d -p tmp)
> diff --git a/scripts/wic_fakeroot b/scripts/wic_fakeroot
> index 88a03fa..16b011e 100755
> --- a/scripts/wic_fakeroot
> +++ b/scripts/wic_fakeroot
> @@ -25,6 +25,11 @@ cmd =3D args[0]
> # rootfs/root ...
> assert os.geteuid() =3D=3D 0, "wic_fakeroot must be run as root!"
> =20
> +# Check if we are calling the pseudo command itself. Return 1
> +# for standalone pseudo operations.
> +if cmd.startswith('-'):
> + sys.exit(0)

I find it hard to match the comment to the code i see. "-" means its
not a cmd but an arg to wic_fakeroot? And what about the 0 vs 1.

Yes. There is an instance[1] where FAK= EROOT, in case of oe the pseudo, is called
as a standalone comman= d with options. We are checking whether the first
option is an ar= gument starting with '-' returning 0 on such calls.

Good catch. I will fix the comment.

[1] https://= github.com/openembedded/openembedded-core/blob/404292b570a78895a1c7900eeb31= 9e36e31dec20/scripts/lib/wic/plugins/source/rootfs.py#L133

Henning

> # e2fsck <=3D 1.43.5 returns 1 on non-errors (stretch and befo= re
> affected) # treat 1 as safe ... the filesystem was successfully
> repaired and is OK if cmd.startswith('fsck.'):

------=_Part_1995_2046633043.1599319145917-- ------=_Part_1994_500397524.1599319145917--