Skip to content

ci: use dev-container for nuttx targets#24832

Merged
mrpollo merged 6 commits intomainfrom
pr-build-with-dev-container
May 16, 2025
Merged

ci: use dev-container for nuttx targets#24832
mrpollo merged 6 commits intomainfrom
pr-build-with-dev-container

Conversation

@alexcekay
Copy link
Copy Markdown
Member

@alexcekay alexcekay commented May 12, 2025

Change

This PR changes all CI workflows that previously used a NuttX container to use the new dev container.
Build all targets was tested here: https://github.com/PX4/PX4-Autopilot/actions/runs/15064605660/job/42346543522?pr=24832
FLASH savings on v5x and v6x are ~44K

Open topics

Tagging strategy

As there is no latest or stable of the dev container yet, I used the most updated dev container. This would thus require updates when new dev containers are created in the future.

Is there a container tagging strategy yet?
Just adding latest automatically on each dev container build workflow run is not a complete solution as this would also trigger when creating a RC for an old PX4 release. I see the following options:

  • Have some magic in the workflow that sets latest to the highest SemVersion
  • Manually set a latest or stable tag for the dev container

@mrpollo: Can you manually create a new updated dev container that we can reference in this PR? Or do you guys prefer another strategy.

Checks not working

Fail 1:
https://github.com/PX4/PX4-Autopilot/actions/runs/14971242279/job/42052457639?pr=24832
This seems to be fixable by adding a safe directory. Does not depend on GCC as it also fails when using the old GCC, thus related to the newer git contained in the dev container.

Fix is contained in this PR.

Fail 2:
https://github.com/PX4/PX4-Autopilot/actions/runs/14971242273/job/42052457618?pr=24832
This seems harder to fix, had a look, but don't see the reason yet. Seems to be a problem with the new version of gcov, so no visible functional problem.

This needs to be fixed in a separate PR as it will require a deeper investigation. Thus I did not use the dev container in the checks.yml.

Flash Analysis not working

https://github.com/PX4/PX4-Autopilot/actions/runs/14971242276/job/42052457576?pr=24832
Known problem with some GCC versions, see here: google/bloaty#362

Easiest fix is not displaying compileunits but symbols instead.
The output will then look like this, which is as helpful as the previous one in my opinion:

    FILE SIZE        VM SIZE    
 --------------  -------------- 
  +0.0%    +296  +0.0%    +296    .text
    +0.8% +1.29Ki  +0.8% +1.29Ki    g_cromfs_image
    [NEW]    +152  [NEW]    +152    land_detector::RoverLandDetector::RoverLandDetector()
    [NEW]    +122  [NEW]    +122    land_detector::RoverLandDetector::_update_topics()
    [NEW]    +120  [NEW]    +120    commands.26190
    +8.2%     +60  +8.2%     +60    AirspeedValidator::check_first_principle()

Fix is contained in this PR.

@alexcekay alexcekay force-pushed the pr-build-with-dev-container branch 4 times, most recently from 9dcdfd8 to f92431d Compare May 12, 2025 12:58
@alexcekay alexcekay marked this pull request as ready for review May 12, 2025 13:10
@alexcekay
Copy link
Copy Markdown
Member Author

FYI: @sfuhrer, @MaEtUgR

@alexcekay alexcekay requested review from dagar and mrpollo May 12, 2025 13:15
Copy link
Copy Markdown
Contributor

@mrpollo mrpollo left a comment

Choose a reason for hiding this comment

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

Thanks @alexcekay, this helps a lot. I intended to review all of the CI jobs and replace them in one single pass, but I was waiting for an updated container tag.

From now on, it will be best if we only push containers for tagged PX4 releases. However, we should have the flexibility to push a container tag on demand, which is why I made #24821. Hopefully we can get it into main soon, and push a new tag for CI

Comment thread Tools/ci/generate_board_targets_json.py Outdated
@dagar
Copy link
Copy Markdown
Member

dagar commented May 14, 2025

Looking good, let's just be sure to review a few flight test logs.

@dagar
Copy link
Copy Markdown
Member

dagar commented May 14, 2025

The flash savings is incredible.

@sfuhrer
Copy link
Copy Markdown
Contributor

sfuhrer commented May 14, 2025

Looking good, let's just be sure to review a few flight test logs.

@dagar anything specific you look out for in flight testing?

@DronecodeBot
Copy link
Copy Markdown

This pull request has been mentioned on Discussion Forum for PX4, Pixhawk, QGroundControl, MAVSDK, MAVLink. There might be relevant details there:

https://discuss.px4.io/t/px4-dev-call-may-14-2025-team-sync-and-community-q-a/45516/3

@DronecodeBot
Copy link
Copy Markdown

This pull request has been mentioned on Discussion Forum for PX4, Pixhawk, QGroundControl, MAVSDK, MAVLink. There might be relevant details there:

https://discuss.px4.io/t/px4-dev-call-may-14-2025-team-sync-and-community-q-a/45516/1

@alexcekay alexcekay marked this pull request as draft May 15, 2025 08:30
@alexcekay alexcekay force-pushed the pr-build-with-dev-container branch from 8e89b54 to f92431d Compare May 15, 2025 08:42
@alexcekay alexcekay marked this pull request as ready for review May 15, 2025 08:46
@alexcekay alexcekay force-pushed the pr-build-with-dev-container branch 5 times, most recently from bd43283 to 6d7b793 Compare May 16, 2025 09:09
@alexcekay
Copy link
Copy Markdown
Member Author

alexcekay commented May 16, 2025

I manually built a new dev-container using the newly action of @mrpollo (thanks 👍). Then I updated my change to use that newly built container.

The build all targets run is here and looks good: https://github.com/PX4/PX4-Autopilot/actions/runs/15064605660/job/42346543522?pr=24832. Also downloaded a build artifact and flashed it onto my v5x, which also looks good (GCC13 + ITCM runs fine).

@alexcekay alexcekay requested review from MaEtUgR and sfuhrer May 16, 2025 09:31
sfuhrer
sfuhrer previously approved these changes May 16, 2025
Copy link
Copy Markdown
Contributor

@sfuhrer sfuhrer left a comment

Choose a reason for hiding this comment

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

I wouldn't know what's holding us back from getting this in, thanks a lot for pushing this forward @alexcekay !

Comment thread Tools/ci/generate_board_targets_json.py Outdated
@alexcekay alexcekay force-pushed the pr-build-with-dev-container branch from 6d7b793 to 317e73b Compare May 16, 2025 09:50
@alexcekay alexcekay requested a review from sfuhrer May 16, 2025 09:54
sfuhrer
sfuhrer previously approved these changes May 16, 2025
@mrpollo
Copy link
Copy Markdown
Contributor

mrpollo commented May 16, 2025

I think I can fix the failed build pretty easily, give me a few to push a couple of commits

Signed-off-by: Ramon Roche <mrpollo@gmail.com>
Signed-off-by: Ramon Roche <mrpollo@gmail.com>
@mrpollo mrpollo force-pushed the pr-build-with-dev-container branch from c29bec7 to 712dc27 Compare May 16, 2025 15:57
pull from github registry ghcr.io

Signed-off-by: Ramon Roche <mrpollo@gmail.com>
@mrpollo mrpollo force-pushed the pr-build-with-dev-container branch from d22dcbe to 0a2eadf Compare May 16, 2025 16:07
@mrpollo mrpollo merged commit 38548de into main May 16, 2025
66 checks passed
@mrpollo mrpollo deleted the pr-build-with-dev-container branch May 16, 2025 16:54
This was referenced May 16, 2025
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.

5 participants