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]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to