Skip to content

Conversation

@btashton
Copy link
Contributor

This updates the GitHub action to use the container that is built by incubator-nuttx-testing. It also update the ESP32 postbuild step to have the binary blob path be configurable making it easier to use pre-built files rather than expecting the whole esp-idf project to be available.

This does require this PR to be merged first apache/nuttx-testing#20

@btashton btashton force-pushed the esp32docker branch 2 times, most recently from ee7d5cf to c0e8b7c Compare March 23, 2020 04:37
matrix:
boards: [arm-01, arm-02, arm-03, arm-04, arm-05, arm-06, arm-07, arm-08, arm-09, arm-10, arm-11, arm-12, arm-13, arm-14, arm-15, mips-riscv-x86, sim]

boards: [arm-01, arm-02, arm-03, arm-04, arm-05, arm-06, arm-07, arm-08, arm-09, arm-10, arm-11, arm-12, arm-13, arm-14, arm-15, mips-riscv-x86, sim, xtensa]
Copy link
Contributor

Choose a reason for hiding this comment

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

Since xtensa's config is very small, let's merge xtensa into mips-riscv-x86?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated to use this instead. Not the CI on this will fail until the PR in testing is merged.

username: ${GITHUB_ACTOR}
password: ${{ secrets.GITHUB_TOKEN }}

- name: Run Pull Container
Copy link
Contributor

Choose a reason for hiding this comment

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

"Run builds already use ci-container at line 104, why we need another similar "Run Pull Container"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is basically zero cost. I left this so that we can see what the time for pull is, I'm worried that this is going to get quite large so I want to be able to keep an eye on it outside of test execution. It will be local after this so the next step does not have to download again.

@@ -0,0 +1,7 @@
name: ci-container
Copy link
Contributor

@xiaoxiang781216 xiaoxiang781216 Mar 28, 2020

Choose a reason for hiding this comment

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

Should we add linux into the name to distinguish the upcoming windows doc?
And we need add copyright in this file too.

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'll add the copyright, but I will probably just expose an input to the action so that it can have the correct container tag used. I would like to wait until we know how that will work, I have not done windows builds in a container via github actions, I have only done them on the runner directly.

@xiaoxiang781216
Copy link
Contributor

xiaoxiang781216 commented Mar 28, 2020

@btashton the patch need rebase the latest mainline, because PR:
#635
apache/nuttx-apps#145

@btashton
Copy link
Contributor Author

@xiaoxiang781216 Should be good now. Let me know if there is anything else.

@xiaoxiang781216 xiaoxiang781216 merged commit f787df1 into apache:master Mar 28, 2020
@btashton btashton deleted the esp32docker branch December 30, 2020 01:10
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.

2 participants