-
Notifications
You must be signed in to change notification settings - Fork 22
Build libraries using Github Actions #78
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
base: main
Are you sure you want to change the base?
Conversation
ebb0c4d
to
c3f0d16
Compare
This seems to need a little tweaking still; It used to work on my fork but when doing PRs across multiple repos, it hits permission issues. |
I'm not really very proficient in CI actions, but with that said, I did not notice anything outstanding. Once you resolve the permission issues, I suggest you push it. One request I have, but that could definitely wait for the future is to also provide builds for the But this can wait a bit. For that, the CI would need to download and install the ARM GCC toolchain. Also, I would soon open a PR which is streamlining a bit the |
Anything we can change in regards to the permission problems? |
- Add the possiblity to build the libraries using Github Actions, and programatically push them to master or a branch - Enable `rebuild-libs` label to force rebuild and push libraries to a PR branch
c3f0d16
to
54b3ef4
Compare
I've tried all possibilities and there isn't really a bypass due to how GitHub handles permissions for security. So what I did, is add a condition to check if the PR CI is executed on the same base repo. If the PR is from a fork to an upstream, it'll still publish the artifacts in the job artifacts (if label is enabled), but it won't commit them. So contributors have 2 possibilities:
Nevertheless, pushing to master still works when we merge because we are doing the push on the same repo |
I don't have those boards and I don't have experience in building for them. I believe most of the CI is architecture agnostic so as you said, once we provide the builder for them, we just need the toolchain installed in CI and it should work. |
982a167
to
3763c6c
Compare
I updated the CI to include the new feature added in: esp-rs/xtensa-toolchain#41 |
This PR adds a long-awaited feature; Build libraries using the CI.
When changes occur in
esp-mbedtls-sys/
, thegen
xtask is triggered to rebuild the libraries, and then tests are executed against the current libraries. When pushing tomain
, the libraries are then committed and pushed to themain
branch by the action.Also, when labelling a PR with the
rebuild-libs
label, the CI is programmed to rebuild libraries and push them to the PR branch. When merging tomain
, since no changes will be detected, there won't be a second commit.This enables rebuilding the libraries without needing to setup the build environment locally on a computer, allows contributors to bring changes to the underlying libraries, while we ensure they are rebuilt in a safe environment, and also allows users to pull back the build libraries to test them locally, if they don't want to set up the environment to build them locally.
Due to the current architecture, executing CI on a push outside of
main
is now disabled. Also, to make the action push the libraries, either the trigger has to be a push tomain
, or adding the labelrebuild-libs
to the PR, since GitHub doesn't support labels on branches directly, so we can't conditionally decide when to push or not, that way.