On 19-01-2024 01:38, Randy MacLeod wrote:
On 2024-01-18 5:57 a.m., yash.shi...@windriver.com wrote:
From: Yash Shinde<yash.shi...@windriver.com>
Enable rust oe-selftest for rust 1.74.1 version and
add target_cfg.patch to handle target configurations
for custom targets.
Hi Yashe,
Yay, getting the rust test suite working again great news!
We talked about this in today's YP bug review and decided NOT to merge
this
until after the M2 build is taken care of over the next few days.
That gives me a chance to review the commit and ask some questions! ;-)
First, your commit log should be expanded to explain what has changed
at a high level
to someone who is just reading "git log".
What you have is a good start but I'd like more info on what changed in:
meta/lib/oeqa/selftest/cases/rust.py
There are lots of tests added to the exclude list and some seem to
have just moved.
Please explain why you did this and if there is any way to avoid the
"noise" of moved tests in future updates. Could you also report on the
number of newly disabled tests and try to summarize in the commit log
what these tests are doing and
Ah, I see what you have done: 1. sort the exclude list 2. add the
newly excluded tests. Please do that in separate commits so that it's
easier to understand. This will reduce the size of the "Enable rust
oe-selftest" diff for meta/lib/oeqa/selftest/cases/rust.py from: | 103
++++++++----- to: | 37 +++-
and let people easily see that most of the newly excluded tests are in
tests/codegen
That reminds me, do we really need to list directories in that exclude
list or only list .rs files?
If I save the list to /tmp/jj, you can see that we could reduce the
list by 34 lines:
❯ rg "\.rs" /tmp/jj | wc -l 203 ❯ rg -v "\.rs" /tmp/jj | wc -l 34
If we can do that, again do it as a separate commit since it should
not affect the outcome of the test.
Also this backport link:
https://github.com/rust-lang/rust/pull/119619/commits
is actually 3 separate commits so it might be better to apply those
individually
so that we can know when they can be dropped by running:
$ git tag --contains <commit-id> on each commit
Also in the patches, please add you Signed-Off-By line but keep the
upstream author's S-O-B line.
I will do patch changes to split into separate commits for each issue
fix, and with oe-selftest changes.
Exclude list is sorted and updated w/ module names, and now the module
names and test list reduced to 114 lines from 201 lines.
I will send a v2 with updating above changes.
Regards,
Yash
For anyone interested we did talk about the number of Rust tests that
we are excluding and of course our goal is to work with the upstream
Rust community to reduce the number of excluded tests over the coming
months but for now, it's just nice to see the test working again!
If you update these things convince yourself that the changes are
functionally identical, I don't think there's any need to re-do all
the testing that you have done.
Thanks Yash,
../Randy
--
# Randy MacLeod
# Wind River Linux
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#194043):
https://lists.openembedded.org/g/openembedded-core/message/194043
Mute This Topic: https://lists.openembedded.org/mt/103805798/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-