Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

trunner: add sparcv8leon3-generic-qemu target support #372

Merged
merged 2 commits into from
Oct 11, 2024

Conversation

maska989
Copy link
Contributor

@maska989 maska989 commented Sep 18, 2024

JIRA: CI-474

Description

Motivation and Context

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Chore (refactoring, style fixes, git/CI config, submodule management, no code logic changes)

How Has This Been Tested?

  • Already covered by automatic testing.
  • New test added: (add PR link here).
  • Tested by hand on: (sparcv8leon3-generic-qemu).

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing linter checks and tests passed.
  • My changes generate no new compilation warnings for any of the targets.

Special treatment

@nalajcie
Copy link
Member

regarding the commit message: this is not a name of any TaRgEt supported by phoenix-RTOS :P

Copy link
Contributor

@damianloew damianloew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please enable this target to CI in a separate commit.

mem/test.yaml Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Sep 18, 2024

Unit Test Results

7 725 tests  ±0   7 010 ✅ ±0   36m 53s ⏱️ -11s
  445 suites ±0     715 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 8b01125. ± Comparison against base commit aba8104.

♻️ This comment has been updated with latest results.

@maska989 maska989 force-pushed the maska989/add_Sparcv8Leon3_General_support branch from 8f3e1a7 to c141281 Compare September 18, 2024 08:59
@maska989 maska989 changed the title trunner: add Sparcv8Leon3_Generic target support trunner: add sparcv8leon3-generic-qemu target support Sep 18, 2024
@maska989 maska989 force-pushed the maska989/add_Sparcv8Leon3_General_support branch 2 times, most recently from 24427c5 to b33c810 Compare September 18, 2024 09:15
@maska989 maska989 requested a review from damianloew September 18, 2024 09:19
Copy link
Member

@mateusz-bloch mateusz-bloch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When building with tests option

LD  test_mmap               
/src/phoenix-rtos-tests/devices/grlib-multi/test-grlib-multi.c:28:2: error: #error "Unsupported target"
   28 | #error "Unsupported target"
      |  ^~~~~
/src/phoenix-rtos-tests/devices/grlib-multi/test-grlib-multi.c:31:10: fatal error: grlib-multi.h: No such file or directory
   31 | #include <grlib-multi.h>
      |          ^~~~~~~~~~~~~~~
compilation terminated.
make: *** [../phoenix-rtos-build/Makefile.common:211: /src/_build/sparcv8leon3-generic-qemu/phoenix-rtos-tests/devices/grlib-multi/test-grlib-multi.o] Error 1
make: *** Waiting for unfinished jobs....

@maska989 maska989 force-pushed the maska989/add_Sparcv8Leon3_General_support branch from b33c810 to f515cfb Compare September 18, 2024 10:53
@mateusz-bloch
Copy link
Member

When building with tests option

LD  test_mmap               
/src/phoenix-rtos-tests/devices/grlib-multi/test-grlib-multi.c:28:2: error: #error "Unsupported target"
   28 | #error "Unsupported target"
      |  ^~~~~
/src/phoenix-rtos-tests/devices/grlib-multi/test-grlib-multi.c:31:10: fatal error: grlib-multi.h: No such file or directory
   31 | #include <grlib-multi.h>
      |          ^~~~~~~~~~~~~~~
compilation terminated.
make: *** [../phoenix-rtos-build/Makefile.common:211: /src/_build/sparcv8leon3-generic-qemu/phoenix-rtos-tests/devices/grlib-multi/test-grlib-multi.o] Error 1
make: *** Waiting for unfinished jobs....

You could add this to the Makefile.sparcvleon3, or come up with better solution

ifneq ($(TARGET), sparcv8leon3-generic-qemu)
DEFAULT_COMPONENTS += test-grlib-multi
endif

@maska989 maska989 force-pushed the maska989/add_Sparcv8Leon3_General_support branch 2 times, most recently from 1d3be02 to 65d206a Compare September 23, 2024 14:03
Makefile.sparcv8leon3 Outdated Show resolved Hide resolved
mem/test.yaml Outdated Show resolved Hide resolved
mem/test.yaml Outdated Show resolved Hide resolved
psh/test.yaml Show resolved Hide resolved
trunner/target/emulated.py Outdated Show resolved Hide resolved
@maska989 maska989 force-pushed the maska989/add_Sparcv8Leon3_General_support branch 3 times, most recently from baa8005 to 2f114c5 Compare September 26, 2024 11:00
psh/test.yaml Outdated Show resolved Hide resolved
@maska989 maska989 force-pushed the maska989/add_Sparcv8Leon3_General_support branch 2 times, most recently from 1998544 to 500f476 Compare October 11, 2024 10:41
Makefile.sparcv8leon3 Show resolved Hide resolved
@maska989 maska989 force-pushed the maska989/add_Sparcv8Leon3_General_support branch 3 times, most recently from 78aa65c to 8b01125 Compare October 11, 2024 11:15
Copy link
Contributor

@damianloew damianloew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some notes about commit descriptions.

As it's described in our Golden Rules

  • describe WHY You're doing the change in the commit message body

So you don't want to repeat what you have added in the commit message body (called also commit description). In your case the reason of adding a new target is rather obvious - we just want to support all of our available targets.

About the second commit sparcv8leon3: prevent from building test-grlib-multi on qemu target - the commit title shows perfectly what is going on in this change, you probably want to add information WHY do we need to exclude it, so a good commit description would be:

This test is not intended to build on this particular target.

@maska989 maska989 force-pushed the maska989/add_Sparcv8Leon3_General_support branch from 8b01125 to a007e5d Compare October 11, 2024 11:31
@maska989 maska989 force-pushed the maska989/add_Sparcv8Leon3_General_support branch from a007e5d to c0b8b5c Compare October 11, 2024 11:43
This test is not intended to be built on this particular target.

JIRA: CI-474
@maska989 maska989 force-pushed the maska989/add_Sparcv8Leon3_General_support branch from c0b8b5c to 3ac9225 Compare October 11, 2024 11:49
@damianloew damianloew merged commit 9bc24b3 into master Oct 11, 2024
34 checks passed
@damianloew damianloew deleted the maska989/add_Sparcv8Leon3_General_support branch October 11, 2024 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants