public inbox for openembedded-core@lists.openembedded.org
 help / color / mirror / Atom feed
From: "Mathieu Dubois-Briand" <mathieu.dubois-briand@bootlin.com>
To: "Daniel Andrade" <dani.barra25@gmail.com>
Cc: "Randy MacLeod" <randy.macleod@windriver.com>,
	<openembedded-core@lists.openembedded.org>,
	<hongxu.jia@windriver.com>, <trevor.woerner@linaro.org>
Subject: Re: [OE-core] [PATCH 0/1] wic: updated fstab does not preserve metadata of the original file
Date: Thu, 25 Sep 2025 10:57:25 +0200	[thread overview]
Message-ID: <DD1R90GRFNVC.24IBY55KSXN0@bootlin.com> (raw)
In-Reply-To: <CA+-=zY2HNd2r-Ux8vUbSU72dGXtPArRtzTaSnHiF0JEvC_jC-Q@mail.gmail.com>

OK! So I misunderstood your previous mail.

I confirm that on master branch the test succeed, but fails if I change
the filesystem to squashfs or btrfs.

On a first glance, it looks like you are right, but I can't say I am
experienced with wic internals. We might want a second opinion here.

If the test is wrong, it has to be fixed. And so probably the tested
code also have to be fixed. I this something you can do?

Thanks,
Mathieu

On Wed Sep 24, 2025 at 6:04 PM CEST, Daniel Andrade wrote:
> Hello Mathieu,
>
> Thank you for the quick answer.
> Ok, I will start from the beginning.
>
> The failing test is supposed to test `--no-fstab-update` flags on wks file.
> According to the understanding of the code, the flag is being passed across
> python scripts either as `self.no_fstab_update`, like in
> scripts/lib/wic/partition.py, or `part.no_fstab_update`, on
> scripts/lib/wic/plugins/source/rootfs.py.
> The issue that is arising from the `oe-selftest -r
> wic.Wic.test_no_fstab_update` is that it is not actually testing it,
> because somehow the assigning of the previous variables is not correct,
> which leads to a later assumption.
> The thing that is really doing the "no update" is the line 134 of
> `scripts/lib/wic/partition.py` where it excludes ext* and msdos partitions
> from falling in the common plugin for Rootfs.
> Because of this, I suggested someone to, in a clean and a different
> environment compared to mine, change the wks file used to `squashfs` fstype
> (or any other supported wic fstype) instead of `ext4` which indeed also
> made the test fail.
> In summary, the test is failing not because of the patch (even though maybe
> something can be improved or fixed later), but because it is just checking
> for ext4 partitions that fall in a different way of approaching fstab while
> the real mechanism is not behaving as desired.
>
> TLDR: The test is getting passed not because it is enforcing
> `--no-fstab-update` but because the partition is ext* or msdos. Changing it
> to squashfs, btrfs or any other wic supported partition, with or without
> the patch, will cause this to fail. This is an extra thing non-related with
> the proposed patch but that is preventing it to behave correctly as
> intended by the test.
>
> Mathieu Dubois-Briand <mathieu.dubois-briand@bootlin.com> escreveu (terça,
> 23/09/2025 à(s) 19:44):
>
>> Hi Daniel,
>>
>> Sorry, but I'm not sure to get your point.
>>
>> I ran the test locally, and it does fail. It also fails if I modify it
>> to use squashfs instead of ext4.
>>
>> My reproduction procedure:
>> git clone https://git.yoctoproject.org/poky-ci-archive -b
>> autobuilder.yoctoproject.org/valkyrie/a-full-2320
>> . oe-init-build-env
>> echo 'SANITY_TESTED_DISTROS = ""' >> conf/local.conf
>> oe-selftest -r wic.Wic.test_no_fstab_update
>>
>> Thanks,
>> Mathieu
>>
>> On Tue Sep 23, 2025 at 5:05 PM CEST, Daniel Andrade wrote:
>> > Good Afternoon,
>> >
>> > I have been trying to figure it out and something seems off.
>> > The specific test you refer to may be hiding a problem or maybe I have my
>> > testbench compromised and I would like your help to test.
>> > The test `test_no_fstab_update ` verifies if the fstab was updated or not
>> > because on the wks file used to do it there is a --no-fstab-update. The
>> > thing is, I think this test is passing because what is forcing it to not
>> > update is a constraint that I removed previously on the patch (the one
>> that
>> > checks if the system is ext* or msdos) and not because the flag is
>> > triggering its intended functionality.
>> > The crosscheck I did was modifying the fstype from ext4 to squashfs and
>> > indeed the test failed.
>> > Can someone also verify this?
>> >
>> > Daniel
>> >
>> > Daniel Andrade <dani.barra25@gmail.com> escreveu (sábado, 13/09/2025
>> à(s)
>> > 11:02):
>> >
>> >> Hello guys, sorry for the delay.
>> >> I have been busy, but I hope to provide you with a fix in the next few
>> >> days.
>> >>
>> >> Sorry about that,
>> >> Daniel
>> >>
>> >> Randy MacLeod <randy.macleod@windriver.com> escreveu (quinta,
>> 11/09/2025
>> >> à(s) 17:56):
>> >>
>> >>> On 2025-09-04 11:58 a.m., Mathieu Dubois-Briand via
>> >>> lists.openembedded.org wrote:
>> >>>
>> >>> On Tue Sep 2, 2025 at 9:05 PM CEST, dani.barra25 via
>> lists.openembedded.org wrote:
>> >>>
>> >>> From: Daniel Andrade <dani.barra25@gmail.com> <dani.barra25@gmail.com>
>> >>>
>> >>> Using `install` in the rootfs plugin forces fstab to be replaced
>> entirely, meaning that even its Inodes will change, leading xattrs and
>> SELinux context stored by pseudo not to be applied.
>> >>> The fix just uses `cp` without preserving attributes from the
>> temporary fstab since none of them are needed, just the content.
>> >>> Same thing happens with the predefined mechanisms for ext4 and msdos.
>> Using debugfs there is no way to replace contents while maintaining
>> metadata, so the approach taken on the path was to remove the different
>> fstab logic for those fstypes and also use the same modified cp command.
>> Reviewing the builds I did it seems to work for all of the fstypes.
>> >>>
>> >>> Another problem is that the timestamp applied to fstab is not the same
>> as every other file. It seems like the `SOURCE_DATE_EPOCH` variable goes to
>> the fallback timestamp (`SOURCE_DATE_EPOCH_FALLBACK`).
>> >>> Since you are using that variable everywhere, it is not the same value
>> as ` REPRODUCIBLE_TIMESTAMP_ROOTFS` under `poky/meta/conf/bitbake.conf`
>> that is applied in every other file.
>> >>>
>> >>> Daniel Andrade (1):
>> >>>   wic: Content of the temporary updated fstab should be copied into the
>> >>>     original not replacing it entirely.
>> >>>
>> >>>  meta/conf/bitbake.conf                   |  4 +++-
>> >>>  scripts/lib/wic/partition.py             | 15 +--------------
>> >>>  scripts/lib/wic/plugins/source/rootfs.py |  4 ++--
>> >>>  3 files changed, 6 insertions(+), 17 deletions(-)
>> >>>
>> >>> Hi Daniel,
>> >>>
>> >>> Thanks for your patch.
>> >>>
>> >>> It looks like it is breaking a test:
>> >>>
>> >>> 2025-09-04 15:28:11,112 - oe-selftest - INFO -
>> wic.Wic.test_no_fstab_update (subunit.RemotedTestCase)
>> >>> 2025-09-04 15:28:11,113 - oe-selftest - INFO -  ... FAIL
>> >>> ...
>> >>> 2025-09-04 15:28:11,114 - oe-selftest - INFO -
>> testtools.testresult.real._StringException: Traceback (most recent call
>> last):
>> >>>   File
>> "/srv/pokybuild/yocto-worker/oe-selftest-armhost/build/meta/lib/oeqa/selftest/cases/wic.py",
>> line 859, in test_no_fstab_update
>> >>>     self.assertEqual(bf_fstab_md5sum, part_fstab_md5sum[1])
>> >>>   File "/usr/lib/python3.12/unittest/case.py", line 885, in assertEqual
>> >>>     assertion_func(first, second, msg=msg)
>> >>>   File "/usr/lib/python3.12/unittest/case.py", line 1251, in
>> assertMultiLineEqual
>> >>>     self.fail(self._formatMessage(msg, standardMsg))
>> >>>   File "/usr/lib/python3.12/unittest/case.py", line 715, in fail
>> >>>     raise self.failureException(msg)
>> >>> AssertionError: 'af3c087d6c9131735c8d1f270a226892' !=
>> '9edb8255abd217fdb20e118833afb856'
>> >>> - af3c087d6c9131735c8d1f270a226892
>> >>> + 9edb8255abd217fdb20e118833afb856
>> >>>
>> https://autobuilder.yoctoproject.org/valkyrie/#/builders/23/builds/2412https://autobuilder.yoctoproject.org/valkyrie/#/builders/35/builds/2268
>> >>>
>> >>> Can you fix it please?
>> >>>
>> >>> Ping?
>> >>>
>> >>> I think this is being tracked by:
>> >>>
>> >>> https://bugzilla.yoctoproject.org/show_bug.cgi?id=15947
>> >>>
>> >>> "WIC does not preserve metadata when updating fstab"
>> >>>
>> >>> Btw, we were just following the "Need Info" process during the bug
>> review
>> >>> meeting
>> >>> so that's why I'm sending this email.
>> >>>
>> >>> ../Randy
>> >>>
>> >>> Thanks,
>> >>> Mathieu
>> >>>
>> >>>
>> >>>
>> >>> -=-=-=-=-=-=-=-=-=-=-=-
>> >>> Links: You receive all messages sent to this group.
>> >>> View/Reply Online (#222962):
>> https://lists.openembedded.org/g/openembedded-core/message/222962
>> >>> Mute This Topic: https://lists.openembedded.org/mt/115043355/3616765
>> >>> Group Owner: openembedded-core+owner@lists.openembedded.org
>> >>> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub
>> [randy.macleod@windriver.com]
>> >>> -=-=-=-=-=-=-=-=-=-=-=-
>> >>>
>> >>>
>> >>>
>> >>> --
>> >>> # Randy MacLeod
>> >>> # Wind River Linux
>> >>>
>> >>>
>>
>>
>>
>>
>> --
>> Mathieu Dubois-Briand, Bootlin
>> Embedded Linux and Kernel engineering
>> https://bootlin.com
>>
>>




-- 
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com



  reply	other threads:[~2025-09-25  8:57 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-02 19:05 [PATCH 0/1] wic: updated fstab does not preserve metadata of the original file dani.barra25
2025-09-02 19:05 ` [PATCH 1/1] wic: Content of the temporary updated fstab should be copied into the original not replacing it entirely dani.barra25
2025-10-11 13:18   ` [OE-core] " Mathieu Dubois-Briand
2025-10-17 11:04     ` dani.barra25
2025-10-17 11:04       ` [PATCH 1/1] Fstab: Fix xattrs not being maintained on fstab file when using wic fstab update funtionalities dani.barra25
2025-10-19 10:02         ` Mathieu Dubois-Briand
     [not found]           ` <20251019153922.27208-1-dani.barra25@gmail.com>
2025-10-19 15:39             ` [PATCH] Fstab xattrs: This update ensures that fstab xattrs are correctly updated without adding a lock to the database dani.barra25
2025-10-20 12:09               ` [OE-core] " Alexander Kanavin
2025-10-20 13:18                 ` Daniel Andrade
2025-10-23 13:04                 ` Ross Burton
2025-10-19 15:41           ` [PATCH 1/1] Fstab: Fix xattrs not being maintained on fstab file when using wic fstab update funtionalities Daniel Andrade
2025-09-04 15:58 ` [OE-core] [PATCH 0/1] wic: updated fstab does not preserve metadata of the original file Mathieu Dubois-Briand
2025-09-11 16:56   ` Randy MacLeod
2025-09-13 10:02     ` Daniel Andrade
2025-09-23 15:05       ` Daniel Andrade
2025-09-23 18:44         ` Mathieu Dubois-Briand
2025-09-24 16:04           ` Daniel Andrade
2025-09-25  8:57             ` Mathieu Dubois-Briand [this message]
2025-09-25 11:11               ` Mathieu Dubois-Briand
2025-09-25 11:40                 ` Daniel Andrade

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DD1R90GRFNVC.24IBY55KSXN0@bootlin.com \
    --to=mathieu.dubois-briand@bootlin.com \
    --cc=dani.barra25@gmail.com \
    --cc=hongxu.jia@windriver.com \
    --cc=openembedded-core@lists.openembedded.org \
    --cc=randy.macleod@windriver.com \
    --cc=trevor.woerner@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox