Add clightning support #21
Conversation
greenaddress
force-pushed
the
cln_test
branch
from
7bd7156 to
16f91f2
Compare
|
ack 16f91f2 |
There was a problem hiding this comment.
Haven't really tried this, but from reviewing the changes it seems to work. I guess the main complication is that the configurator is not really well-suited for cross-compilation targets, since it assumes the platform it runs on is the platform the final binary will run on, and there is no way to sideload the config from another system yet.
Some of these patches should be upstreamed, which would reduce the maintenance burden and improve c-lightning's own support. I added comments to that effect where necessary 😉
All in all excellent work @lvaccaro, that's quite some sleuthing to get all the piece to fit 👍
| sed -i 's/$CC ${CWARNFLAGS-$BASE_WARNFLAGS} $CDEBUGFLAGS $COPTFLAGS -o $CONFIGURATOR $CONFIGURATOR.c/$CONFIGURATOR_CC ${CWARNFLAGS-$BASE_WARNFLAGS} $CDEBUGFLAGS $COPTFLAGS -o $CONFIGURATOR $CONFIGURATOR.c/g' configure | ||
| sed -i 's/-Wno-maybe-uninitialized/-Wno-uninitialized/g' configure | ||
| ./configure CONFIGURATOR_CC=${CONFIGURATOR_CC} --prefix=${LNBUILDROOT} --disable-developer --disable-compat --disable-valgrind --enable-static |
There was a problem hiding this comment.
This would be a nice patch to upstream. The configurator is always a bit weird in cross-compile scenarios fwiw
| sed "s'NDKCOMPILER'${CC}'" /repo/lightning-config.vars > config.vars | ||
| sed "s'NDKCOMPILER'${CC}'" /repo/lightning-config.h > ccan/config.h |
There was a problem hiding this comment.
Strange switch of sed separators. Any particular reason for not continuing to use / like above?
There was a problem hiding this comment.
the code used to contain slashes in sed, that was fixed but the separator wasn't changed
There was a problem hiding this comment.
ack, sed syntax could be aligned in the entire script. I don't really like using sed on pre-generated headers file, but it is a dirty trick to don't use qemu in the CI.
| /* Needed for Glibc like endiness check */ | ||
| +#ifndef __LITTLE_ENDIAN | ||
| #define __LITTLE_ENDIAN 1234 | ||
| +#endif | ||
| +#ifndef __BIG_ENDIAN | ||
| #define __BIG_ENDIAN 4321 | ||
| +#endif |
There was a problem hiding this comment.
What is this change required for? It might need upstreaming as well, since we should be able to detect endiannes reliably.
There was a problem hiding this comment.
Good point we should try to give it a go without
There was a problem hiding this comment.
Configurator has an option about HAVE_BIG_ENDIAN, but I am not sure solve the issue during building ccan https://github.com/ElementsProject/lightning/blob/359433f374472a6128993bbaace2c1c7c62ee107/ccan/tools/configurator/configurator.c#L174
There was a problem hiding this comment.
The issue imho is that the configurator is running on the host architecture and will thus use the host endianness instead of the target arch's endianness. Maybe ElementsProject/lightning#3464 could be used to run the configurator in qemu emulating the target arch, thus picking up the correct values automatically?
| - strcpy(addr.sun_path, rpc_filename); | ||
| + addr.sun_path[0] = '\0'; | ||
| + strcpy(addr.sun_path + 1, rpc_filename); | ||
| addr.sun_family = AF_UNIX; |
There was a problem hiding this comment.
I don't quite follow this change. addr.sun_path now has a string terminator at the beginning?
There was a problem hiding this comment.
This patch is not related to the building process, but related to use c-lightning in an android device that doesn't support communication using traditional AF_UNIX socket file. The '\0' swap allows to use abstract namespace socket for inter-process communication (also available on linux kernel that holds in its memory space). I think there are some security concerns on accessing them.
| -CFLAGS = $(CPPFLAGS) $(CWARNFLAGS) $(CDEBUGFLAGS) $(COPTFLAGS) -I $(CCANDIR) $(EXTERNAL_INCLUDE_FLAGS) -I . -I/usr/local/include $(FEATURES) $(COVFLAGS) $(DEV_CFLAGS) -DSHACHAIN_BITS=48 -DJSMN_PARENT_LINKS $(PIE_CFLAGS) $(COMPAT_CFLAGS) -DBUILD_ELEMENTS=1 | ||
| +CFLAGS = $(CPPFLAGS) $(CWARNFLAGS) $(CDEBUGFLAGS) $(COPTFLAGS) -I $(CCANDIR) $(EXTERNAL_INCLUDE_FLAGS) -I . -I$(PREFIX)/include $(FEATURES) $(COVFLAGS) $(DEV_CFLAGS) -DSHACHAIN_BITS=48 -DJSMN_PARENT_LINKS $(PIE_CFLAGS) $(COMPAT_CFLAGS) -DBUILD_ELEMENTS=1 |
|
|
|
commit lvaccaro@68fe1b3 should align sed commands in order to use the same syntax |
|
FWIW I tried this and succesfully ran C-lightning on my phone. I'll submit a patchset for the new version. |
|
@greenaddress ping |
No description provided.