-
Notifications
You must be signed in to change notification settings - Fork 13.3k
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
Subrepo axtls #3424
Subrepo axtls #3424
Conversation
subrepo: subdir: "tools/sdk/axtls" merged: "27f4a6c" upstream: origin: "https://github.com/igrr/axtls-8266" branch: "master" commit: "27f4a6c" git-subrepo: version: "0.3.1" origin: "???" commit: "???"
…tation, Hash library uses axtls crypto library
Thank you for working on this!
axTLS is also used in Sming project: https://github.com/SmingHub/Sming/tree/develop/Sming/third-party.
Removing copied code is a great idea!
There have been some modifications. Some of them trivial, some more intrusive. I am still able to cherry-pick changes from upstream whenever they happen, with most conflicts being resolved automatically, so we haven't diverged from upstream that much.
How did you achieve this? AFAICT Arduino's platform.txt still uses libaxtls.a. To compile axTLS source automatically it would have to be placed into the core directory. Alternatively an approach similar to LwIP could be used, where platform.txt instructs arduino to run 'make' in axTLS directory, then copies the library file over. However this method needs 'make' to be installed, which is not normally the case on Windows. |
The libaxtls.a/make issue (See @igrr last set of comments) is still a thing, and to be honest, I've not touched it in a while. @igrr I achieved it by modifying platform.txt (see: https://github.com/esp8266/Arduino/pull/3424/files#diff-d638a47d1ca0e2a5585fb34c7d7fb0f9) - Not sure that required make? I may pick it up again, as I've just started another ESP8266 project. |
@devyte I'll fix the conflicts in the next couple of days. |
The axtls lib and related wifi socket classes have been deprecated in favor of the bearssl lib and related wifi socket classes. Axtls is still available as legacy for compat in case apps somehow can't migrate to bssl, but it will be retired eventually. |
I was about to get cracking again on #3105 but based on the feedback from @igrr and the comments on #3223 I thought it worth tackling bringing AxTLS in to the build tree.
Using a submodule would work, but they are error prone (forgetting to sync up the submodules is a pain), so I thought about using a subtree, but as I was refreshing my memory on how to set them up, I came across this subrepo project: https://github.com/ingydotnet/git-subrepo#readme. Have a read through, though the TL;DR is there is no extra software or things to worry about for developer just working on the top level repo (ie almost everbody), and there is just an additional script forr those working with the axtls repo.
I've added in https://github.com/igrr/axtls-8266 (in the tools/sdk/axtls directory) and replaced the sha1 implementation in Hash.cpp to use the one from the library instead of the standalone implementation that is currently used.
I've also hacked the library so it would compile when using WifiClientSecure, however it's become clear that some code from the axtls has already been manually copied over, and there has been a few other modifications to the library that may be better located in the core tree.
IMO it would makes sense to have the axtls library as close to the sourceforce version, to make updating easier, so any ESP8266 specific stuff should be in core (where possible).
So my questions:
is the intention of igrr/axtls-8266 to bring axtls to this project, or is it intended for non-arduino projects too?
How do we feel about removing the copied code, and using the version from igrr/axtls-8266?
Has there been heaps of modifications to the existing axtls library?
I wouldn't mind having a bit of a discussion before committing time to cleaning this all up.