ewpa / LibSSH-ESP32

Libssh SSH client & server port to ESP32 Arduino library
https://www.ewan.cc/node/157
Other
257 stars 36 forks source link

Library uncompilable in PlatformIO, weirdness with `config.h` #7

Closed maxgerhardt closed 2 years ago

maxgerhardt commented 3 years ago

Per https://community.platformio.org/t/libssh-esp32-library-examples-not-compiling/21446/.

When using this libraray with PlatformIO, e.g. using the exec and keygen2 examples, a lot of warnings regarding redefinitions appear (stemming from conflicts with config.h), and ultimately errors are thrown regarding the FALL_THROUGH macro and undefined references to ssh_mutex_lock and related functions.

In file included from Z:/Projects/ESP_SSH/src/exec.ino:84:0:
Z:/Projects/ESP_SSH/src/exec.ino: In function 'int verify_knownhost(ssh_session)':
.pio\libdeps\tinypico\LibSSH-ESP32\src/libssh/priv.h:401:24: error: expected primary-expression before '__attribute__'
 #  define FALL_THROUGH __attribute__ ((fallthrough))
Linking .pio\build\tinypico\firmware.elf
.pio\build\tinypico\lib599\libLibSSH-ESP32.a(init.c.o):(.literal._ssh_init+0xc): undefined reference to `ssh_mutex_lock'
.pio\build\tinypico\lib599\libLibSSH-ESP32.a(init.c.o):(.literal._ssh_init+0x10): undefined reference to `ssh_mutex_unlock'
.pio\build\tinypico\lib599\libLibSSH-ESP32.a(init.c.o): In function `_ssh_init':
Z:\Projects\ESP_SSH/.pio\libdeps\tinypico\LibSSH-ESP32\src/init.c:155: undefined reference to `ssh_mutex_lock'
Z:\Projects\ESP_SSH/.pio\libdeps\tinypico\LibSSH-ESP32\src/init.c:155: undefined reference to `ssh_mutex_unlock'
.pio\build\tinypico\lib599\libLibSSH-ESP32.a(threads.c.o):(.literal.ssh_threads_init+0x8): undefined reference to `ssh_threads_get_default'
.pio\build\tinypico\lib599\libLibSSH-ESP32.a(threads.c.o): In function `ssh_threads_init':
Z:\Projects\ESP_SSH/.pio\libdeps\tinypico\LibSSH-ESP32\src/threads.c:55: undefined reference to `ssh_threads_get_default'
collect2.exe: error: ld returned 1 exit status
*** [.pio\build\tinypico\firmware.elf] Error 1

I confirmed that the library compiles in the Arduino IDE with the latest framework version.

However, this may be due a subtle include order difference between PlatformIO and the Arduino IDE, ultimately boiling down to a maybe error in the library.

When you e.g. use the exec example, it does

https://github.com/ewpa/LibSSH-ESP32/blob/e15200e305b3b0f0c3763b71ebdacc9cddd01bf9/examples/exec/exec.ino#L77-L77

which I expect to pull in the config.h from the library, that is,

https://github.com/ewpa/LibSSH-ESP32/blob/e15200e305b3b0f0c3763b71ebdacc9cddd01bf9/src/config.h#L1-L4

However, if you open the exec.ino example in the Arduino IDE and place the following code after all the #includes have happened

#ifdef PORT_ESP32_CONFIG_H_
#error "PORT_ESP32_CONFIG_H_ is defined"
#else
#error "PORT_ESP32_CONFIG_H_ is not defined"
#endif

(that is, to check whether the macro in the config.h header file was activated as we expect to), you will get

exec:95:2: error: #error "PORT_ESP32_CONFIG_H_ is not defined"
 #error "PORT_ESP32_CONFIG_H_ is not defined"
  ^

indidicating that not this config.h that we thought it loaded was actually loaded.

Doing the exact same in PlatformIO (after a few config.h tweaks to get it compilable at all), the opposite occurs

C:/Users/Max/temp/folder_test/src/exec:95:2: error: #error "PORT_ESP32_CONFIG_H_ is defined"
 #error "PORT_ESP32_CONFIG_H_ is defined"
  ^

The file config.h also exists in the Arduino-ESP32 core (see https://github.com/espressif/arduino-esp32/find/1.0.6 and type config.h), so maybe there is some confusion going on as to which file (e.g., library code, example code) is actually loaded when #include "config.h" is done.

The most interesting part is when I rename the config.h of this repository to something unique, say libssh_config.h, and replace all #include "config.h" accordingly (using sed), I get the exact same compiler errors as I get in PlatformIO.

C:\Users\Max\AppData\Local\Temp\arduino_build_669303\libraries\LibSSH-ESP32_Mod\LibSSH-ESP32_Mod.a(init.c.o):(.literal._ssh_init+0xc): undefined reference to `ssh_mutex_lock'
C:\Users\Max\AppData\Local\Temp\arduino_build_669303\libraries\LibSSH-ESP32_Mod\LibSSH-ESP32_Mod.a(init.c.o):(.literal._ssh_init+0x10): undefined reference to `ssh_mutex_unlock'
C:\Users\Max\AppData\Local\Temp\arduino_build_669303\libraries\LibSSH-ESP32_Mod\LibSSH-ESP32_Mod.a(init.c.o): In function `_ssh_init':
C:\Users\Max\Documents\Arduino\libraries\LibSSH-ESP32_Mod\src/init.c:155: undefined reference to `ssh_mutex_lock'
C:\Users\Max\Documents\Arduino\libraries\LibSSH-ESP32_Mod\src/init.c:155: undefined reference to `ssh_mutex_unlock'
C:\Users\Max\AppData\Local\Temp\arduino_build_669303\libraries\LibSSH-ESP32_Mod\LibSSH-ESP32_Mod.a(init.c.o): In function `_ssh_finalize':
C:\Users\Max\Documents\Arduino\libraries\LibSSH-ESP32_Mod\src/init.c:182: undefined reference to `ssh_mutex_lock'
C:\Users\Max\Documents\Arduino\libraries\LibSSH-ESP32_Mod\src/init.c:180: undefined reference to `ssh_mutex_unlock'
C:\Users\Max\AppData\Local\Temp\arduino_build_669303\libraries\LibSSH-ESP32_Mod\LibSSH-ESP32_Mod.a(init.c.o): In function `is_ssh_initialized':
C:\Users\Max\Documents\Arduino\libraries\LibSSH-ESP32_Mod\src/init.c:231: undefined reference to `ssh_mutex_lock'
C:\Users\Max\Documents\Arduino\libraries\LibSSH-ESP32_Mod\src/init.c:278: undefined reference to `ssh_mutex_unlock'
C:\Users\Max\AppData\Local\Temp\arduino_build_669303\libraries\LibSSH-ESP32_Mod\LibSSH-ESP32_Mod.a(threads.c.o):(.literal.ssh_threads_init+0x8): undefined reference to `ssh_threads_get_default'
C:\Users\Max\AppData\Local\Temp\arduino_build_669303\libraries\LibSSH-ESP32_Mod\LibSSH-ESP32_Mod.a(threads.c.o): In function `ssh_threads_init':
C:\Users\Max\Documents\Arduino\libraries\LibSSH-ESP32_Mod\src/threads.c:85: undefined reference to `ssh_threads_get_default'
collect2.exe: error: ld returned 1 exit status

so for some code files, the wrong config.h may actually be loaded which magically works out in the Arduino IDE but not on PlatformIO, but you should take care that the right file will always be loaded anyways -- and if it is, it causes errors though. (See referenced topic).

ewpa commented 3 years ago

Thank you for your initial investigation on this. I have not tested with PlatformIO but your comments indicate some subtle issue is present regardless of the environment. I had hoped to keep code as close to upstream as possible but understand that is not possible sometimes.

maxgerhardt commented 3 years ago

For reference: I've uploaded a compilable PlatformIO example at https://github.com/maxgerhardt/pio-libssh-example with the changes I talked about above, which are in the fork https://github.com/maxgerhardt/LibSSH-ESP32

ewpa commented 3 years ago

I pulled your changes locally and quickly checked it compiled and linked. I would like to refactor to make it clear the files are effectively 'from' libssh_esp32 versus libssh. I will also assess my porting process to prevent any regression, and run internal tests.

ewpa commented 3 years ago

Please could you let me know why the following change is needed so I may investigate further?

diff --git a/src/libssh/libssh.h b/src/libssh/libssh.h
index 0624295..efff4fc 100644
--- a/src/libssh/libssh.h
+++ b/src/libssh/libssh.h
@@ -67,7 +67,9 @@
   #include <winsock2.h>
 #else /* _WIN32 */
  #include <sys/select.h> /* for fd_set * */
- #include <netdb.h>
+ #undef IPADDR_NONE
+ #include <lwip/sockets.h>
+ #undef IPADDR_NONE
 #endif /* _WIN32 */
maxgerhardt commented 3 years ago

Remove these changes, try the exec example and you'll see why :D

In file included from C:\Users\Max\.platformio\packages\framework-arduinoespressif32\tools\sdk\include\lwip/lwip/ip_addr.h:43:0,
                 from C:\Users\Max\.platformio\packages\framework-arduinoespressif32\tools\sdk\include\lwip/lwip/sockets.h:46,
                 from .pio\libdeps\esp32dev\LibSSH-ESP32\src/libssh/libssh.h:71,
                 from src\exec.cpp:25:
C:\Users\Max\.platformio\packages\framework-arduinoespressif32\tools\sdk\include\lwip/lwip/ip4_addr.h:79:37: error: expected ')' before numeric constant
 #define IPADDR_NONE         ((u32_t)0xffffffffUL)
                                     ^
C:\Users\Max\.platformio\packages\framework-arduinoespressif32\tools\sdk\include\lwip/lwip/inet.h:71:29: note: in expansion of macro 'IPADDR_NONE'
 #define INADDR_NONE         IPADDR_NONE
                             ^
C:\Users\Max\.platformio\packages\framework-arduinoespressif32\cores\esp32/IPAddress.h:94:17: note: in expansion of macro 'INADDR_NONE'
 const IPAddress INADDR_NONE(0, 0, 0, 0);
                 ^

lwip defines the macro IPADDR_NONE to 0xfffffff but the Arduino-ESP32 core creates a variable called INADDR_NONE but if the macro would exist at that point the variable declaration would read, in expanded form,

 const IPAddress ((u32_t)0xffffffffUL)(0, 0, 0, 0);

which is ofc invalid. Hence I try to get rid of that asap by undef-ing if.

Actually I tested this again and the more minimal form

#ifdef _WIN32
  #include <winsock2.h>
#else /* _WIN32 */
 #include <sys/select.h> /* for fd_set * */
 #include <netdb.h>
 #undef IPADDR_NONE
#endif /* _WIN32 */

works too.

The exec example has this problem because after it includes libssh (which includes this internal lwip header) it does #include "IPv6Address.h" which pulls in IPAddress.h from the Arduino core, with the macro activated that would lead to the error above.

On a sidenote: The fork is just a proof-of-concept, there's a few points in there that are not good, like:

ewpa commented 3 years ago

Please could you test with the feature-platformio branch and feed your results back:

youssefIOT commented 3 years ago

Hi ALL I have the same issue reported by Maximilian Gerhardt but in ESP-IDF . After successfully ported the code from arduino to ESP-ID. I hope there is some fix for that

/home/ammar/.espressif/tools/xtensa-esp32-elf/esp-2020r3-8.4.0/xtensa-esp32-elf/bin/../lib/gcc/xtensa-esp32-elf/8.4.0/../../../../xtensa-esp32-elf/bin/ld: esp-idf/arduino/libarduino.a(init.c.obj):(.literal._ssh_init+0xc): undefined reference to ssh_mutex_lock' /home/ammar/.espressif/tools/xtensa-esp32-elf/esp-2020r3-8.4.0/xtensa-esp32-elf/bin/../lib/gcc/xtensa-esp32-elf/8.4.0/../../../../xtensa-esp32-elf/bin/ld: esp-idf/arduino/libarduino.a(init.c.obj):(.literal._ssh_init+0x10): undefined reference tossh_mutex_unlock' /home/ammar/.espressif/tools/xtensa-esp32-elf/esp-2020r3-8.4.0/xtensa-esp32-elf/bin/../lib/gcc/xtensa-esp32-elf/8.4.0/../../../../xtensa-esp32-elf/bin/ld: esp-idf/arduino/libarduino.a(init.c.obj): in function _ssh_init': /home/ammar/esp/SSH_TO_IDF/build/../components/arduino/libraries/LibSSH-ESP32/src/init.c:66: undefined reference tossh_mutex_lock' /home/ammar/.espressif/tools/xtensa-esp32-elf/esp-2020r3-8.4.0/xtensa-esp32-elf/bin/../lib/gcc/xtensa-esp32-elf/8.4.0/../../../../xtensa-esp32-elf/bin/ld: /home/ammar/esp/SSH_TO_IDF/build/../components/arduino/libraries/LibSSH-ESP32/src/init.c:91: undefined reference to ssh_mutex_unlock' /home/ammar/.espressif/tools/xtensa-esp32-elf/esp-2020r3-8.4.0/xtensa-esp32-elf/bin/../lib/gcc/xtensa-esp32-elf/8.4.0/../../../../xtensa-esp32-elf/bin/ld: esp-idf/arduino/libarduino.a(init.c.obj): in function_ssh_finalize': /home/ammar/esp/SSH_TO_IDF/build/../components/arduino/libraries/LibSSH-ESP32/src/init.c:104: undefined reference to ssh_mutex_lock' /home/ammar/.espressif/tools/xtensa-esp32-elf/esp-2020r3-8.4.0/xtensa-esp32-elf/bin/../lib/gcc/xtensa-esp32-elf/8.4.0/../../../../xtensa-esp32-elf/bin/ld: /home/ammar/esp/SSH_TO_IDF/build/../components/arduino/libraries/LibSSH-ESP32/src/init.c:185: undefined reference tossh_mutex_unlock' /home/ammar/.espressif/tools/xtensa-esp32-elf/esp-2020r3-8.4.0/xtensa-esp32-elf/bin/../lib/gcc/xtensa-esp32-elf/8.4.0/../../../../xtensa-esp32-elf/bin/ld: esp-idf/arduino/libarduino.a(threads.c.obj):(.literal.ssh_threads_init+0x8): undefined reference to ssh_threads_get_default' /home/ammar/.espressif/tools/xtensa-esp32-elf/esp-2020r3-8.4.0/xtensa-esp32-elf/bin/../lib/gcc/xtensa-esp32-elf/8.4.0/../../../../xtensa-esp32-elf/bin/ld: esp-idf/arduino/libarduino.a(threads.c.obj): in functionssh_threads_init': /home/ammar/esp/SSH_TO_IDF/build/../components/arduino/libraries/LibSSH-ESP32/src/threads.c:55: undefined reference to `ssh_threads_get_default' collect2: error: ld returned 1 exit status ninja: build stopped: subcommand failed. ninja failed with exit code 1

thank you

maxgerhardt commented 3 years ago

undefined reference to ssh_mutex_unlock

This is fixed by commenting out the PTHREAD macro in the config header file, see my branch linked above.

Sorry I haven't been able to look at @ewpa branch above yet, I hope I can do that rather soon.

youssefIOT commented 3 years ago

thank you Maximilian ! do you mean tcommenting out his define ? / Define to 1 if you have the header file. /

define HAVE_PTHREAD_H 1

maxgerhardt commented 3 years ago

No, see here as I've also linked above.

youssefIOT commented 3 years ago

Ok , i will try this changes. thank you again i try it it works !. but i run with new issue with argp_parse :-)

ewpa commented 3 years ago

Please raise a new issue for argp_parse.

ewpa commented 3 years ago

Please test release 1.3.0, commit 4f3da87. I intend to close this issue shortly.