Skip to content
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

ESP-IDF 5.1.1 compile fail #29

Open
VNovytskyi opened this issue Sep 21, 2023 · 16 comments
Open

ESP-IDF 5.1.1 compile fail #29

VNovytskyi opened this issue Sep 21, 2023 · 16 comments
Assignees
Labels
enhancement New feature or request

Comments

@VNovytskyi
Copy link

Brief: Can not compile the library with ESP-IDF project.
ESP-IDF version: 5.1.1
LibSSH-ESP32 version: 3.1.0
Errors list:

  • LibSSH-ESP32/src/libmbedcrypto.c:196:47: error: 'mbedtls_md_context_t' has no member named 'md_info'
  • LibSSH-ESP32/src/libssh/poll.h:42:27: error: conflicting types for 'nfds_t'; have 'long unsigned int'

Notes:

  1. I think the problem with nfds_t is related to the ESP32 definition. I don't properly understand where I need to provide the #define ESP32 because there are two configuration files (libssh_esp32_config.h)
  2. The problem with md_info can be fixed by the next way:
    *len = (unsigned int)mbedtls_md_get_size(c->MBEDTLS_PRIVATE(md_info));

Request: Test the LibSSH-ESP32 with ESP-IDF 5.1.1

@ewpa
Copy link
Owner

ewpa commented Sep 21, 2023

I tested builds only with ESP-IDF 4.4.5, since that is the version used with the Arduino framework. Let me have the commit id from the official espressif/arduino-esp32.git repository so this can be reproduced.

@ewpa ewpa added the enhancement New feature or request label Sep 21, 2023
@VNovytskyi
Copy link
Author

VNovytskyi commented Sep 22, 2023

I do not use the Arduino framework. I have a project based on the ESP-IDF framework. I downloaded the last version of the ESP-IDF from the official site (5.1.1) and got the last version of LibSSH-ESP32 (3.1.0). You can use my test repository (https://github.com/VNovytskyi/libssh-esp32-idf511-build) to try the build. This repository was created only to test the build process. I'd like to suggest adding the CMakeList.txt file to simplify the building process for non-Arduino projects. In the test project, you can see a lot of warnings/errors. These errors would be good to clean up.

@ewpa
Copy link
Owner

ewpa commented Sep 23, 2023

Awesome, thanks! When building against ESP-IDF v5.1.1 using idf.py all I'm flooded with the below error. What might be different with my build environment?

error: format '%u' expects argument of type 'unsigned int', but argument 5 has type 'uint32_t' {aka 'long unsigned int'} [-Werror=format=]

@VNovytskyi
Copy link
Author

VNovytskyi commented Sep 24, 2023

It is the first error that would be great to fix. It is an error that occurs in a lot of source files in LibSSH-ESP32. This error appeared because the ESP-IDF 5.1.1 started using the GCC 12.2.0, instead of the old GCC 8.4.0. You can read more information about this at https://docs.espressif.com/projects/esp-idf/en/latest/esp32/migration-guides/release-5.x/5.0/index.html and https://docs.espressif.com/projects/esp-idf/en/latest/esp32/migration-guides/release-5.x/5.1/index.html.

I propose the following solution to this problem:

set_source_files_properties(
  ../libs/LibSSH-ESP32/src/auth.c
  ../libs/LibSSH-ESP32/src/agent.c
  ../libs/LibSSH-ESP32/src/dh-gex.c
  ../libs/LibSSH-ESP32/src/channels.c
  
  PROPERTIES
    COMPILE_FLAGS -Wno-format
)

If we are using this solution we steel with backward compatibility (as I think, it needs to be tested). Another way to fix this problem is to replace the %u with %ul (the GCC 11 compiler must help you with hints about errors). In the case of this fix, case may be problems with backward compabilities in the future.

The next error is missing the MBEDTLS_PRIVATE macro in some cases and changing the call of some Mbed TLS functions. This error occurs because the ESP-IDF 5.1.1 updates the version of the Mbed TLS library from v2.x to v3.1.0.

It is not a lot of a problem to update the LibSSH-ESP32 for ESP-IDF 5. You can feel free to ask me about some issues when you update the library. I hope the next release of LibSSH-ESP32 will have ESP-IDF 5 support.

@ewpa
Copy link
Owner

ewpa commented Oct 22, 2023

The following patch is applied to src/libssh/poll.h during porting:

-typedef unsigned long int nfds_t;
+#if !defined ESP32 || ESP_IDF_VERSION_MAJOR < 4
+typedef unsigned long int nfds_t;
+#endif /* ESP32 */

Therefore if you set ESP32 that particular issue should be resolved. GCC compilation errors addressed in version 4.0.0+ of this library (not released yet, supports Arduino 3.0.0 but breaks Arduino 1.0.6).

@SylvianHemus
Copy link

The following patch is applied to src/libssh/poll.h during porting:

-typedef unsigned long int nfds_t;
+#if !defined ESP32 || ESP_IDF_VERSION_MAJOR < 4
+typedef unsigned long int nfds_t;
+#endif /* ESP32 */

Therefore if you set ESP32 that particular issue should be resolved. GCC compilation errors addressed in version 4.0.0+ of this library (not released yet, supports Arduino 3.0.0 but breaks Arduino 1.0.6).

I'm still getting the same problem after deleting the conditional block and just having it set. long unsigned int in auth.c

@SylvianHemus
Copy link

SylvianHemus commented Dec 9, 2023

Fixed variables for recent GCC version, see:

SylvianHemus@84a7589

will make pull request. Need to fix the mbedtls_md_context issue. Espressif provides the following guide I'll follow
https://github.com/espressif/mbedtls/blob/9bb5effc3298265f829878825d9bd38478e67514/docs/3.0-migration-guide.md

@SylvianHemus
Copy link

Created pull request, compiles now but mostly untested
#30

@ewpa
Copy link
Owner

ewpa commented Dec 10, 2023

Created pull request, compiles now but mostly untested

The limitation of PRs is they do not address the re-porting needs when a new release of upstream libssh is available. I had not pushed the work I had done on this a couple of months ago. Now pushed, hence please refer to release 4.0.1 in new branch preview-arduino-esp32-3.0.0 commit 1a11d61 and advise.

@SylvianHemus
Copy link

Created pull request, compiles now but mostly untested

The limitation of PRs is they do not address the re-porting needs when a new release of upstream libssh is available. I had not pushed the work I had done on this a couple of months ago. Now pushed, hence please refer to release 4.0.1 in new branch preview-arduino-esp32-3.0.0 commit 1a11d61 and advise.

Oh nice thanks, I'll try this and report back

@SylvianHemus
Copy link

SylvianHemus commented Dec 15, 2023

Created pull request, compiles now but mostly untested

The limitation of PRs is they do not address the re-porting needs when a new release of upstream libssh is available. I had not pushed the work I had done on this a couple of months ago. Now pushed, hence please refer to release 4.0.1 in new branch preview-arduino-esp32-3.0.0 commit 1a11d61 and advise.

I tested, I get returned mbedtls_md_context_t has no member md_info on branch preview-arduino-esp32-3.0.0. Since the update you can't access it with that function any longer I think, a lot of properties are now private. Have to use something like *len = (unsigned int)mbedtls_md_get_size(mbedtls_md_info_from_ctx(c)); see bdf5429 , 2f0cca5

@ewpa
Copy link
Owner

ewpa commented Dec 24, 2023

I tested, I get returned mbedtls_md_context_t has no member md_info on branch preview-arduino-esp32-3.0.0. Since the update you can't access it with that function any longer I think, a lot of properties are now private. Have to use something like *len = (unsigned int)mbedtls_md_get_size(mbedtls_md_info_from_ctx(c)); see bdf5429 , 2f0cca5

Looking at this line of code, it comes from upstream. The best way to approach a fix is to get and apply a patch for libssh to use the newer mbedtls 3.1 (currently max 3.0 supported for 0.10 stable). However in any case, code for both versions of mbedtls will need to remain in order to support IDF 4.4 and 5 for Arduino.

@iodeo
Copy link

iodeo commented Jun 17, 2024

Hi,
Just to point out that ESP32-Arduino core version 3 has been released with ESP-IDF 5.1.4 behind it.
This causes the following error (same as above) using Arduino IDE:
LibSSH-ESP32/src/libmbedcrypto.c:196:47: error: 'mbedtls_md_context_t' has no member named 'md_info'

@ewpa
Copy link
Owner

ewpa commented Jun 18, 2024

Acknowledged. I do not think fixing the compilation error is enough since I'd also expect further errors in regards to the new version of mbedtls in IDF 5. I believe support for the new mbedtls version is being added into the libssh master branch now, so maybe a re-port will be the solution (just for core 3.0.1+).

@ewpa ewpa self-assigned this Jun 23, 2024
@ewpa
Copy link
Owner

ewpa commented Jun 23, 2024

re-port will be the solution

I have pushed a preview release, branch preview-arduino-esp32-3.0.0 commit 917fc3d for you to test. I ran only basic tests and found no issues under 2.0.17 or 3.0.1. Let me know how you get on.

@ewpa
Copy link
Owner

ewpa commented Sep 28, 2024

Fixed with commit 057ada8. Please check and confirm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants