-
-
Notifications
You must be signed in to change notification settings - Fork 62
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
Patch to allow use of precompiled libraries #101
Conversation
All the check failed on 27th April with this error (don't know if it was after the CI fix merging): Warning: This version of the action is deprecated. Use arduino/compile-sketches. See https://github.com/arduino/compile-sketches |
@landret the CI has been fixed but the pull request needs to be rebased on top of it. |
Thanks for the reply. How do I rebase it? Do I have to do a new pull request? |
@landret no, it's not necessary to do a new pull request. You can see a tutorial about it here: https://akrabat.com/the-beginners-guide-to-rebasing-your-pr/ Note that in the tutorial the pull request base branch is named It's not possible to do this process via the GitHub web interface. The tutorial is for using Git from the command line. Some Git clients also provide the ability to do it via a GUI, but that process will depend on which Git client you happen to be using. Please let me know if you have any problems. |
Done, thanks. Now it looks like the workflow needs approval for running here. |
Great work @landret! The workflow needing approval is the result of a recent policy change from GitHub to prevent malicious abuse of GitHub Actions, and not specific to this PR or the repository. I don't have the necessary permissions here to approve the workflow runs. Maybe @facchinm can help us out with that. |
Memory usage change @ 991866d
Click for full report table
Click for full report CSV
|
CI output is a bit difficult to interpret but it's failing on Tweeter example from https://github.com/arduino-libraries/Arduino_OAuth/tree/master/examples/Tweeter lib due to low resources. @per1234 should we exclude that example (or library) from the CI? |
For me, there is an excessive amount of output. I would propose removing the "verbose output during compilation" setting from the CI workflow:
That option is primarily intended for use when troubleshooting the action or build system. I did that in the SAMD boards platform repo a couple months ago (arduino/ArduinoCore-samd#617) and haven't heard any complaints from people missing the verbose output.
If the sketch is intended to be usable with these boards, and the work needed to restore usability is related to this repository's code, then I would say it should stay in order to track the problem. If the sketch is not intended to be usable with these boards, then it should be removed. I'm happy to adjust the CI configuration according to your preferences. Just let me know. |
I'm super ok with removing the verbosity; for the example, I think it started failing for external reasons due to some dependency becoming too resource hungry.
WiFiNINA is the only one that was heavily updated lately, and maybe the ArduinoBearSSL dependency is a side effect of a commit there. Investigating 🕵️ |
So, after bisecting, it looks like the culprit is this PR arduino-libraries/ArduinoBearSSL#34 ; probably the avr compile is not smart enough to strip the singletons ( |
Not sure. We've got |
Hi guys. Have the CI problems been solved ? |
Regarding this proposal I mentioned above:
The PR is here: #104 But that is only intended to make the logs easier to read to understand the cause of a failed compilation. |
The CI error should be fixed by https://github.com/arduino-libraries/ArduinoBearSSL/releases/tag/1.7.1 , so as soon as the library is picked up by the library manager we can restart the job and then merge the PR. |
ldflags added to allow precompilation.
991866d
to
71437e7
Compare
Memory usage change @ 71437e7
Click for full report table
Click for full report CSV
|
Hi @facchinm I rebased the PR on top and all checks have passed now. |
Hi,
based on issues #100 and to allow use of precompiled libraries, I added the same patch as arduino/ArduinoCore-avr@bca2493 to the platform.txt.
This patch was successfully tested with Nano Every and Uno Wifi Rev2.