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.

Sign up for GitHub

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

Jump to bottom

ci: update actions and images #24039

Merged
merged 1 commit into from
Dec 4, 2024
Merged

ci: update actions and images #24039

merged 1 commit into from
Dec 4, 2024

Conversation

mrpollo
Copy link
Contributor

@mrpollo mrpollo commented Nov 25, 2024

Problem
GitHub deprecated older versions of the cache, and checkout actions which are now using an updated version of node which isnt' available on the older PX4 container images

Solution
Updated cache, and checkout deps to latest where possible, and switched to the newer px4-dev container image with the updated base operating system environment

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 25, 2024 •
edited
Loading

FLASH Analysis

px4_fmu-v5x 
    FILE SIZE        VM SIZE    
--------------  -------------- 
[ = ]       0  [ = ]       0    TOTAL
px4_fmu-v6x 
    FILE SIZE        VM SIZE    
--------------  -------------- 
[ = ]       0  [ = ]       0    TOTAL

Updated: 2024-12-04T17:39:40

@mrpollo mrpollo force-pushed the mrpollo/fix_gha_deps branch from 4bfe36e to 258080d Compare November 26, 2024 17:49
@DronecodeBot
Copy link

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-sync-q-a-nov-27-2024/42604/1

@mrpollo mrpollo force-pushed the mrpollo/fix_gha_deps branch from 0b28622 to f7630d4 Compare November 27, 2024 20:34
@mrpollo mrpollo changed the title ci: update actions and images DONT MERGE (yet) -- ci: update actions and images Nov 27, 2024
@mrpollo mrpollo force-pushed the mrpollo/fix_gha_deps branch from affa6e2 to 5268a89 Compare November 27, 2024 22:00
@dagar dagar marked this pull request as draft November 27, 2024 22:41
@mrpollo mrpollo force-pushed the mrpollo/fix_gha_deps branch from 5ab2531 to dbe8ca5 Compare November 28, 2024 14:59
@mrpollo mrpollo changed the title DONT MERGE (yet) -- ci: update actions and images ci: update actions and images Nov 28, 2024
@mrpollo mrpollo mentioned this pull request Nov 28, 2024
Merged
@mrpollo mrpollo force-pushed the mrpollo/fix_gha_deps branch 3 times, most recently from 1df7e85 to e467171 Compare November 28, 2024 19:23
@mrpollo mrpollo mentioned this pull request Nov 29, 2024
Closed
@mrpollo mrpollo force-pushed the mrpollo/fix_gha_deps branch 5 times, most recently from d36d194 to 6b8c2f8 Compare December 4, 2024 16:33
mrpollo
mrpollo commented Dec 4, 2024
View reviewed changes
Copy link
Contributor Author

@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.

hey @dagar can you take a look? i think its ready for review

.github/workflows/checks.yml
@@ -25,28 +25,27 @@ jobs:
"NO_NINJA_BUILD=1 px4_fmu-v5_default",
"NO_NINJA_BUILD=1 px4_sitl_default",
"px4_sitl_allyes",
"airframe_metadata",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed airframe_metadata & parameters_metadata from this workflow, they are also being built by the "Build all targets" workflow. I don't see a point in building again here.

.github/workflows/ekf_update_change_indicator.yml
- name: auto-commit any changes to change indication
uses: stefanzweifel/git-auto-commit-action@v4
with:
commit_message: '[AUTO COMMIT] update change indication'
file_pattern: 'src/modules/ekf2/test/change_indication/*.csv'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

check ekf files only

.github/workflows/ekf_update_change_indicator.yml
commit_message: |
'[AUTO COMMIT] update change indication'

See .github/workflopws/ekf_update_change_indicator.yml for more details
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found a few of these commits in the codebase, they are hard to follow, adding this comment will help someone point to the source

.github/workflows/failsafe_sim.yml
- uses: actions/checkout@v1
with:
token: ${{ secrets.ACCESS_TOKEN }}
- name: Install Node v20.18.0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Installing node directly on the container for this test.

Why? the install emscripten is giving me tons of issues when running with the docker-run package

.github/workflows/failsafe_sim.yml
- name: Install Node v20.18.0
uses: actions/setup-node@v4
with:
node-version: 20.18.0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the version required by the emscripten dependency

@DronecodeBot
Copy link

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-sync-q-a-dec-4-2024/42715/1

@mrpollo
ci: update actions and images
8190241 
Updates some actions to run build steps in container instead of the
whole workflow.
@mrpollo mrpollo force-pushed the mrpollo/fix_gha_deps branch from 6b8c2f8 to 8190241 Compare December 4, 2024 17:34
@mrpollo mrpollo requested a review from dagar December 4, 2024 17:35
@mrpollo mrpollo marked this pull request as ready for review December 4, 2024 17:36
@mrpollo mrpollo mentioned this pull request Dec 4, 2024
Open
@dagar dagar merged commit 2848735 into main Dec 4, 2024
59 checks passed
@dagar dagar deleted the mrpollo/fix_gha_deps branch December 4, 2024 17:59
@MaEtUgR
Copy link
Member

MaEtUgR commented Dec 5, 2024

Thank you @mrpollo ! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dagar dagar dagar approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@mrpollo @DronecodeBot @MaEtUgR @dagar