The (first ?) troublemaker during compilation are calls to functions in sha256.c
There are some deprecated functions which are replaced like
void mbedtls_sha256_starts which is
mbedtls_sha256_starts_ret now.
As far as I can see, in Espruino they they are used for Crypto.

Looks like ESP uses https://github.com/espressif/esp-idf/blob/master/components/mbedtls/port/include/mbedtls/esp_config.h

Probably we could have a config.h outside of the directory and just tweak the include paths?

I'm not against changing mbedtls_sha256_starts to mbedtls_sha256_starts_ret and so on if it means we can use the newer version.

However I'd prefer not to use the submodule approach. If you updated as part of the build it'd mean that you could no longer tell exactly which Espruino you got based on the commit hash - Travis builds would pass and then mbedtls would get updated and it'd all break again.

I think this time I'd just include the new mbedtls - maybe it's something we could tweak later but I'd prefer not to change too much.

@gfwilliams

Probably we could have a config.h outside of the directory and just tweak the include paths?
That sound good.

However I'd prefer not to use the submodule approach
With submodules - it does not automatically update. You need to actively advance to the next version.

We're using it for the build tools. You pin the folder to be a link to release you want to use:

https://github.com/espruino/EspruinoBuildTools/tree/master/esp32/build

It works like a symbolic link in unix:
esp-idf @ c2b39f4 Update SDK to esp-idf 3.0.1

So this is pinned to the 3.0.1 release.

Ahh, sorry - yeah, I do this as part of EspruinoWebIDE.

I notice there is one local change in there apart from config.h, but it may have been fixed in the actual mbedtls by now.

As long as it doesn't cause problems with the Travis build then let's try it then. It is nice to reduce the amount of code in the repo - I just don't want to do anything that makes the build less reliable :)

Great.

If the config.h is out of tree where would config.h go?

https://github.com/espruino/Espruino/tree/master/libs/crypto

Looks like we define this to the config.h path

Espruino/tree/master/libs/crypto/mbedtls_config

At least that's the form I take with the nRF5x SDK

Would that be a .h file or a folder?

Well, since we can actually make it a file with MBEDTLS_CONFIG_FILE, let's just do Espruino/tree/master/libs/crypto/mbedtls_config.h.

That's way nicer than having a random config.h file on our include path

Ok, so step one would be to get the existing build working with the new #define and the config in it's new place. Then as a second step swap out to a new version, and the new update the changed functions.

Later on for the esp32 build, we can also look at using the ESP-idf build version of the library, as this has hardware assist.

I've checked one more option.
Idea was to have minor changes in Espruino and at the same time open a way for board specific mbedtls.

• copied crypto handling as default to a new directory (make/crypto)
• changed crypto handling in Makefile

ifdef USE_CRYPTO
ifdef USE_ESP32
  include make/crypto/$(FAMILY).make
else
  include make/crypto/default.make
endif
endif

• created an ESP32 specific make file in crypto directory

DEFINES += -DUSE_CRYPTO
CRYPTODIR = $(ESP_IDF_PATH)/components/mbedtls
INCLUDE += -I$(ROOT)/libs/crypto
INCLUDE += -I$(CRYPTODIR)
INCLUDE += -I$(CRYPTODIR)/mbedtls/include
INCLUDE += -I$(CRYPTODIR)/mbedtls/include/mbedtls
LDFLAGS += -L$(ESP_APP_TEMPLATE_PATH)/build/mbedtls
LIBS += -lmbedtls
WRAPPERSOURCES += libs/crypto/jswrap_crypto.c
ifdef USE_SHA256
  DEFINES += -DUSE_SHA256
endif
ifdef USE_SHA512
  DEFINES += -DUSE_SHA512
endif
ifdef USE_TLS
  USE_AES=1
  DEFINES += -DUSE_TLS
endif
ifdef USE_AES
  DEFINES += -DUSE_AES
endif

• created template for ESP32, without hardware acceleration. With hardware acceleration on, some strange errors appear. There is an idea how to fix it, lets see in the future.

Before spending more time on that, I would like to wait for feedback ;-)

Sorry, closed it by accident

Or even a better way to avoid board specific naming in makefile

ifdef USE_CRYPTO
  CRYPTOFILE = make/crypto/$(FAMILIY).make
  ifeq ($(wildcard $(CRYPTOFILE)),)
    include make/crypto/$(FAMILY).make
  else
    include make/crypto/default.make
  endif
endif

^^^ That looks good.

Only thing I'd say is to try and avoid as much duplication as possible (eg ifdef USE_TLS etc is probably the same on absolutely all platforms?)

Ok, I take it back - it's different :) Yeah, I'd do as you suggest then. Nice to move all of that out of Makefile too :)

The existing build for ESP32 uses the espruino mbedtls. So why does this need to change for the 3.1 release? I’m missing something here.

Somewhere in ESP-IDF mbedtls functions are called. To be honest, I've no idea where and why.
These function calls are not available in older version of mbedtls, used in Espruino.
Calls used in Espruino are marked as depreciated in more actual version of mbedtls used in ESP-IDF.
Result is, that during linking we get some strange errors.
The only option to get it compiled without problems, is with changes mentioned above.
At least, it's the only one I found.

#1560
#1533