-
Notifications
You must be signed in to change notification settings - Fork 58
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
Some patch suggestions #53
Open
micmac1
wants to merge
3
commits into
signalwire:master
Choose a base branch
from
micmac1:ow-up
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Currently the search paths for math.h are set statically. This results in being unable to compile libks in a buildroot when cross-compiling. <snip> -- Using unsigned short -- Check if the system is big endian - big endian -- Could NOT find LibM (missing: LIBM_INCLUDE_DIRS) <snip> -- Configuring done CMake Error at CMakeLists.txt:332 (add_library): Target "ks" links to target "LIBM::LIBM" but the target was not found. Perhaps a find_package() call is missing for an IMPORTED target, or an ALIAS target is missing? <snip> Fix this by allowing standard search paths. Signed-off-by: Sebastian Kemper <sebastian_ml@gmx.net>
The include should read <signal.h> instead of <sys/signal.h>. Some toolchains show a warning when the include is wrong: In file included from /home/sk/tmp/openwrt/build_dir/target-mips_24kc_musl/libks-2019-09-18-df72c4c6/src/include/libks/ks_platform.h:94:0, from /home/sk/tmp/openwrt/build_dir/target-mips_24kc_musl/libks-2019-09-18-df72c4c6/src/include/libks/ks.h:32, from /home/sk/tmp/openwrt/build_dir/target-mips_24kc_musl/libks-2019-09-18-df72c4c6/src/ks.c:23: /home/sk/tmp/openwrt/staging_dir/toolchain-mips_24kc_gcc-7.4.0_musl/include/sys/signal.h:1:2: warning: #warning redirecting incorrect #include <sys/signal.h> to <signal.h> [-Wcpp] #warning redirecting incorrect #include <sys/signal.h> to <signal.h> ^~~~~~~ This commit fixes the include. Signed-off-by: Sebastian Kemper <sebastian_ml@gmx.net>
Currently ksutil_setup_platform() is called before project(). That's why distro (or user) compiler flags are not used. When you want to make amendments to the flags you need to do so _after_ project(). [ 4%] Building C precompiled header cotire/ks_C_prefix.h.gch cc1: note: someone does not honour COPTS correctly, passed 0 times Scanning dependencies of target ks make[5]: Leaving directory '/home/sk/tmp/openwrt/build_dir/target-mips_24kc_musl/libks-2019-09-18-df72c4c6' make[5]: Entering directory '/home/sk/tmp/openwrt/build_dir/target-mips_24kc_musl/libks-2019-09-18-df72c4c6' [ 5%] Building C object CMakeFiles/ks.dir/src/include/cJSON/cJSON.c.o cc1: note: someone does not honour COPTS correctly, passed 0 times [ 7%] Building C object CMakeFiles/ks.dir/src/include/cJSON/cJSON_Utils.c.o cc1: note: someone does not honour COPTS correctly, passed 0 times More explicit example: /home/sk/tmp/openwrt/staging_dir/toolchain-mips_24kc_gcc-7.4.0_musl/bin/mips-openwrt-linux-musl-gcc -DHAVE_BYTESWAP_H=1 -DHAVE_CLOCK_GETRES=1 -DHAVE_CLOCK_GETTIME=1 -DHAVE_CLOCK_NANOSLEEP=1 -DHAVE_DIRENT_H=1 -DHAVE_DLFCN_H=1 -DHAVE_INTTYPES_H=1 -DHAVE_LIBCRYPTO=1 -DHAVE_LIBSSL=1 -DHAVE_MALLOC=1 -DHAVE_MEMMEM=1 -DHAVE_MEMORY_H=1 -DHAVE_PTHREAD_SETSCHEDPARAM=1 -DHAVE_SCHED_H=1 -DHAVE_SCHED_SETSCHEDULER=1 -DHAVE_STDINT_H=1 -DHAVE_STDLIB_H=1 -DHAVE_STRFTIME=1 -DHAVE_STRINGS_H=1 -DHAVE_STRING_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_UNISTD_H=1 -DHAVE_USLEEP=1 -DKS_PLAT_LIN=1 -DRETSIGTYPE=void -DSTDC_HEADERS=1 -DTIME_WITH_SYS_TIME=1 -D_REENTRANT=1 -D__BYTE_ORDER=__BIG_ENDIAN -I/home/sk/tmp/openwrt/build_dir/target-mips_24kc_musl/libks-2019-09-18-df72c4c6/tests -I/home/sk/tmp/openwrt/build_dir/target-mips_24kc_musl/libks-2019-09-18-df72c4c6/src/include -I/home/sk/tmp/openwrt/build_dir/target-mips_24kc_musl/libks-2019-09-18-df72c4c6 -std=c11 -DNDEBUG -fPIE -O2 -g -std=gnu99 -o CMakeFiles/testq.dir/tap.c.o -c /home/sk/tmp/openwrt/build_dir/target-mips_24kc_musl/libks-2019-09-18-df72c4c6/tests/tap.c This was seen while CMAKE_C_FLAGS:STRING=-Os -pipe -mno-branch-likely -mips32r2 -mtune=24kc -fno-caller-saves -fno-plt -fhonour-copts -Wno-error=unused-but-set-variable -Wno-error=unused-result -msoft-float -mips16 -minterlink-mips16 -iremap/home/sk/tmp/openwrt/build_dir/target-mips_24kc_musl/libks-2019-09-18-df72c4c6:libks-2019-09-18-df72c4c6 -Wformat -Werror=format-security -fstack-protector -D_FORTIFY_SOURCE=1 -Wl,-z,now -Wl,-z,relro -I/home/sk/tmp/openwrt/staging_dir/target-mips_24kc_musl/usr/include -I/home/sk/tmp/openwrt/staging_dir/target-mips_24kc_musl/include -I/home/sk/tmp/openwrt/staging_dir/toolchain-mips_24kc_gcc-7.4.0_musl/usr/include -I/home/sk/tmp/openwrt/staging_dir/toolchain-mips_24kc_gcc-7.4.0_musl/include/fortify -I/home/sk/tmp/openwrt/staging_dir/toolchain-mips_24kc_gcc-7.4.0_musl/include was set in CMakeCache.txt (distro flags). So none of the distro flags were used. Fix this by rearanging rearranging CMakeLists.txt a bit. Signed-off-by: Sebastian Kemper <sebastian_ml@gmx.net>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi all,
Some suggestions after toying around with libks.
One other thing, I think the cmake files should probably not be installed in /usr/include/libks/cmake. I think "$LIBDIR"/cmake/libks is probably correct. But then you'd also need to update the users, like signalwire-c.
Regards,
Seb
P.S.: Feel free to close this if you would like to solve this some other way.