| Message ID | 20200902185624.15044-7-Vijaikumar_Kanagarajan@mentor.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | WIC update | expand |
On Thu, 3 Sep 2020 00:26:20 +0530 Vijai Kumar K <Vijaikumar_Kanagarajan@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. > > 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 <Vijaikumar_Kanagarajan@mentor.com> > --- > 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. > > # 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. 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.'):
wic_fakeroot says that it can eventually be replaced with "true", which would make this patch obsolete. And probably the python symlinking from the previous patch. The stretch debian package has gotten a few updates since, and jessie is not support by isar anymore. So maybe worth checking if the fsck hack is still needed, and therefore the whole script. Also what wic calls might have changed, or it might itself "ignore" the problem that my code seems to work around there. Same goes for "export MTOOLS_SKIP_CHECK=1", that might not be required anymore. and should be double-checked. Henning On Thu, 3 Sep 2020 00:26:20 +0530 Vijai Kumar K <Vijaikumar_Kanagarajan@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. > > 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 <Vijaikumar_Kanagarajan@mentor.com> > --- > 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 > > # 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) > + > # 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.'):
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 <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. > > > > 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 <Vijaikumar_...@mentor.com> > > --- > > 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.'): > >
On Saturday, September 5, 2020 at 2:49:26 PM UTC+5:30 Henning Schild wrote: > wic_fakeroot says that it can eventually be replaced with "true", which > would make this patch obsolete. And probably the python symlinking from > the previous patch. > > The stretch debian package has gotten a few updates since, and jessie > is not support by isar anymore. So maybe worth checking if the fsck > hack is still needed, and therefore the whole script. Also what wic > calls might have changed, or it might itself "ignore" the problem that > my code seems to work around there. > When I tested, this was not the case, stretch would still need this fsck hack to work. > > Same goes for "export MTOOLS_SKIP_CHECK=1", that might not be required > anymore. and should be double-checked. > Not either. We would still need this at least for stretch I think. I will check again though just to verify my observation back then. > > Henning > > > 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. > > > > 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 <Vijaikumar_...@mentor.com> > > --- > > 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 > > > > # 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) > > + > > # 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.'): > >
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 # 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) + # 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.'):
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 <Vijaikumar_Kanagarajan@mentor.com> --- meta/classes/wic-img.bbclass | 1 + scripts/wic_fakeroot | 5 +++++ 2 files changed, 6 insertions(+)