ntop / n2n

Peer-to-peer VPN
GNU General Public License v3.0
6.14k stars 928 forks source link

compile error help on openwrt #1041

Closed galaxyskyknight closed 1 year ago

galaxyskyknight commented 2 years ago

I recently encounter the compile issue on LEDE openwrt with n2n, for the latest code here 06c489fd8ad42d6c025beaea0fd62d7d4d948c31, I got the following logs:

/home/builder/lede_x86_5_15/build_dir/target-x86_64_musl/n2n-3.1.1_dev_git-06c489fd8ad42d6c025beaea0fd62d7d4d948c31/src/tf.c:75:15: note: shadowed declaration is here
 const uint8_t Q1[] = { 0x75, 0xF3, 0xC6, 0xF4, 0xDB, 0x7B, 0xFB, 0xC8, 0x4A, 0xD3, 0xE6, 0x6B, 0x45, 0x7D, 0xE8, 0x4B,
               ^~
[25/157] Building C object CMakeFiles/n2n.dir/src/management.c.o
FAILED: CMakeFiles/n2n.dir/src/management.c.o
/home/builder/lede_x86_5_15/staging_dir/toolchain-x86_64_gcc-8.4.0_musl/bin/x86_64-openwrt-linux-musl-gcc -DCMAKE_BUILD -DHAVE_MINIUPNP -DHAVE_NATPMP -DHAVE_OPENSSL_1_1 -DHAVE_PCAP_IMMEDIATE_MODE -DHAVE_PTHREAD -DHAVE_ZSTD -DMINIUPNP_STATICLIB -DPACKAGE_OSNAME=\"Linux\" -DPACKAGE_VERSION=\"3.1.1\" -I/home/builder/lede_x86_5_15/build_dir/target-x86_64_musl/n2n-3.1.1_dev_git-06c489fd8ad42d6c025beaea0fd62d7d4d948c31/thirdparty/miniupnp/miniupnpc/include -I/home/builder/lede_x86_5_15/build_dir/target-x86_64_musl/n2n-3.1.1_dev_git-06c489fd8ad42d6c025beaea0fd62d7d4d948c31/thirdparty/libnatpmp -I/home/builder/lede_x86_5_15/build_dir/target-x86_64_musl/n2n-3.1.1_dev_git-06c489fd8ad42d6c025beaea0fd62d7d4d948c31/lib_miniupnpc -I/home/builder/lede_x86_5_15/build_dir/target-x86_64_musl/n2n-3.1.1_dev_git-06c489fd8ad42d6c025beaea0fd62d7d4d948c31/. -I/home/builder/lede_x86_5_15/build_dir/target-x86_64_musl/n2n-3.1.1_dev_git-06c489fd8ad42d6c025beaea0fd62d7d4d948c31/include -Os -pipe -fno-caller-saves -fno-plt -fhonour-copts -Wno-error=unused-but-set-variable -Wno-error=unused-result -fmacro-prefix-map=/home/builder/lede_x86_5_15/build_dir/target-x86_64_musl/n2n-3.1.1_dev_git-06c489fd8ad42d6c025beaea0fd62d7d4d948c31=n2n-3.1.1_dev_git-06c489fd8ad42d6c025beaea0fd62d7d4d948c31 -Wformat -Werror=format-security -fstack-protector -D_FORTIFY_SOURCE=1 -Wl,-z,now -Wl,-z,relro -Wall -Wshadow -Wpointer-arith -Wmissing-declarations -Wnested-externs -O2 -DNDEBUG -MD -MT CMakeFiles/n2n.dir/src/management.c.o -MF CMakeFiles/n2n.dir/src/management.c.o.d -o CMakeFiles/n2n.dir/src/management.c.o -c /home/builder/lede_x86_5_15/build_dir/target-x86_64_musl/n2n-3.1.1_dev_git-06c489fd8ad42d6c025beaea0fd62d7d4d948c31/src/management.c
In file included from /home/builder/lede_x86_5_15/build_dir/target-x86_64_musl/n2n-3.1.1_dev_git-06c489fd8ad42d6c025beaea0fd62d7d4d948c31/src/management.h:11,
                 from /home/builder/lede_x86_5_15/build_dir/target-x86_64_musl/n2n-3.1.1_dev_git-06c489fd8ad42d6c025beaea0fd62d7d4d948c31/src/management.c:17:
/home/builder/lede_x86_5_15/build_dir/target-x86_64_musl/n2n-3.1.1_dev_git-06c489fd8ad42d6c025beaea0fd62d7d4d948c31/include/n2n_typedefs.h:630:5: error: unknown type name 'pthread_t'
     pthread_t               id;            /* thread id */
     ^~~~~~~~~
/home/builder/lede_x86_5_15/build_dir/target-x86_64_musl/n2n-3.1.1_dev_git-06c489fd8ad42d6c025beaea0fd62d7d4d948c31/include/n2n_typedefs.h:631:5: error: unknown type name 'pthread_mutex_t'
     pthread_mutex_t         access;        /* mutex for shared access */
     ^~~~~~~~~~~~~~~
ninja: build stopped: subcommand failed.
make[2]: *** [Makefile:93: /home/builder/lede_x86_5_15/build_dir/target-x86_64_musl/n2n-3.1.1_dev_git-06c489fd8ad42d6c025beaea0fd62d7d4d948c31/.built] Error 1
make[2]: Leaving directory '/home/builder/lede_x86_5_15/package/lean/n2n'
time: package/lean/n2n/compile#12.98#1.44#18.23
    ERROR: package/lean/n2n failed to build.
make[1]: *** [package/Makefile:116: package/lean/n2n/compile] Error 1
make[1]: Leaving directory '/home/builder/lede_x86_5_15'
make: *** [/home/builder/lede_x86_5_15/include/toplevel.mk:230: package/lean/n2n/compile] Error 2

however, I use the same makefile to compile the commit on f3e305b254fc88ce829ddb4a63b11e083a65c3ab with no any issue and compile success!

the following is the Makefile I used, can any one of you help to check why the latest code say there is no pthread define and how to fix that? thanks!

# SPDX-License-Identifer: GPL-3.0-only
#
# Copyright (C) 2020 - ntop.org and contributors
# Copyright (C) 2021-2022 ImmortalWrt.org

include $(TOPDIR)/rules.mk

PKG_NAME:=n2n
PKG_VERSION:=3.1.1
PKG_RELEASE:=$(AUTORELEASE)

PKG_SOURCE_PROTO:=git
PKG_SOURCE_URL:=https://github.com/ntop/n2n.git
PKG_SOURCE_VERSION:=06c489fd8ad42d6c025beaea0fd62d7d4d948c31
#PKG_SOURCE_VERSION:=f3e305b254fc88ce829ddb4a63b11e083a65c3ab
PKG_VERSION:=3.1.1_dev_git-$(PKG_SOURCE_VERSION)

PKG_LICENSE:=GPL-3.0
PKG_LICENSE_FILE:=LICENSE
PKG_MAINTAINER:=Emanuele Faranda <faranda@ntop.org>

include $(INCLUDE_DIR)/package.mk
include $(INCLUDE_DIR)/cmake.mk

define Package/n2n/template
  SECTION:=net
  CATEGORY:=Network
  SUBMENU:=VPN
  TITLE:=N2N Peer-to-peer VPN
  URL:=http://www.ntop.org/n2n
  DEPENDS:=+libcap +libopenssl +libpthread +libzstd
endef

define Package/n2n
  $(call Package/n2n/template)
  DEPENDS+=+kmod-tun +resolveip +libminiupnpc +libnatpmp
endef

define Package/n2n/description
  This package contains client node and supernode for the N2N infrastructure.
endef

define Package/n2n/conffiles
/etc/config/n2n
endef

define Package/n2n-utils
  $(call Package/n2n/template)
  DEPENDS+=+n2n +libpcap
endef

define Package/n2n-utils/description
  This package contains extend utilities for the N2N infrastructure.
endef

CMAKE_OPTIONS+= \
        -DCMAKE_BUILD_TYPE=Release \
        -DN2N_OPTION_USE_PTHREAD=ON \
        -DN2N_OPTION_USE_OPENSSL=ON \
        -DN2N_OPTION_USE_PCAPLIB=ON \
        -DN2N_OPTION_USE_ZSTD=ON \
        -DN2N_OPTION_USE_PORTMAPPING=ON \
        -Wno-dev

define Package/n2n/install
        $(INSTALL_DIR) $(1)/usr/bin
        $(INSTALL_BIN) $(PKG_INSTALL_DIR)/usr/sbin/edge $(1)/usr/bin/n2n-edge
        $(INSTALL_BIN) $(PKG_INSTALL_DIR)/usr/sbin/supernode $(1)/usr/bin/n2n-supernode

        $(INSTALL_DIR) $(1)/etc/config
        $(INSTALL_CONF) ./files/n2n.config $(1)/etc/config/n2n
        $(INSTALL_DIR) $(1)/etc/init.d
        $(INSTALL_BIN) ./files/n2n.init $(1)/etc/init.d/n2n
        $(INSTALL_DIR) $(1)/etc/uci-defaults
        $(INSTALL_BIN) ./files/n2n-migrate-conf.sh $(1)/etc/uci-defaults/50-n2n-migrate-conf
endef

define Package/n2n-utils/install
        $(INSTALL_DIR) $(1)/usr/bin
        $(INSTALL_BIN) $(PKG_INSTALL_DIR)/usr/bin/n2n-benchmark $(1)/usr/bin/
        $(INSTALL_BIN) $(PKG_INSTALL_DIR)/usr/bin/n2n-decode $(1)/usr/bin/
        $(INSTALL_BIN) $(PKG_BUILD_DIR)/n2n-keygen $(1)/usr/bin/
endef

$(eval $(call BuildPackage,n2n))
$(eval $(call BuildPackage,n2n-utils))
hamishcoleman commented 2 years ago

Are you able to reproduce this with our known working openwrt build process we have included in the repo? That will make it simpler to check

hamishcoleman commented 2 years ago

I kicked off an automated CI build with our standard build process and it successfully builds. Can you explain why you are using a different build process? Perhaps if you can outline the improvements achieved with your diffierent build process we can consider adopting them in the standard build.

However, for the moment, I would suggest you build with the standard build process because it can successfully compile the code.

galaxyskyknight commented 2 years ago

I kicked off an automated CI build with our standard build process and it successfully builds. Can you explain why you are using a different build process? Perhaps if you can outline the improvements achieved with your diffierent build process we can consider adopting them in the standard build.

However, for the moment, I would suggest you build with the standard build process because it can successfully compile the code.

I am not using the official Openwrt but the LEDE repo, this makefile comes from the LEDE repo. I believe your stand process is working for the official openwrt repo but I never use it as I will stick to the LEDE repos and follow its makefile. that's the reason.

btw: can we directly face to the real problem? it is straightforward, it is working for the previous commit but not any more for the latest, there must be some problem there for the recent change(may be compatible issue), it will be more helpful to me to get a quick solution than tangled with a process critical, I would apperciate that if you could help on this, thanks for your understand.

galaxyskyknight commented 2 years ago

I tired this makefile on the commit one by one start where I could work it through, and here is the findings, and later on commtis till to the latest there are some different errors but pthread error is always there. image

galaxyskyknight commented 2 years ago

I add the #include <pthread.h> at the head of n2n_typedefs.h(originally I add n2n.h but introduce the other cross header include issue, it is problem and incorrect, so the bestway is to include pthread.h directly), the issue resolved for me, the compile smoothly passed, please consider the fix and try the CI to see if it has any problem on other env, thanks. @hamishcoleman

https://github.com/galaxyskyknight/n2n/commit/199714e73a5f8fa798bddff8c68294f97d431932

hamishcoleman commented 2 years ago

Thanks for narrowing down what the problem is. We will try and have a look at where we can include that header (n2n_typedefs.h is probably not the right place)

Can you tell me what the difference you get between OpenWRT and LEDE is? According to the OpenWRT about page, they have merged with LEDE...

galaxyskyknight commented 2 years ago

Thanks for narrowing down what the problem is. We will try and have a look at where we can include that header (n2n_typedefs.h is probably not the right place)

Can you tell me what the difference you get between OpenWRT and LEDE is? According to the OpenWRT about page, they have merged with LEDE...

Hi there, I am sorry that I am not using the OpenWRT official repo, so I have no idea what their makefile done, but for lede repo, they are recently upgrade to 3.0 and newly developed the makefile accordingly and changed to use CMake as you can see in this makefile, I believe this is a minor problem since I can add it in n2n_typedefs.h and no other compile issue at now, I am asking for help here is just to verify it is good to your 'standard' process is not broken with adding this include file and merge it to the dev as a fix if it is fine.

thanks.

hamishcoleman commented 2 years ago

I found the LEDE repository and it appears to be almost identical to the OpenWRT repository. Perhaps I am looking at the wrong place, can you point me at the repository that you are compiling from. New applications of this code base should not be using CMake as that is less maintained, less understood, less flexible and most importantly it is deprecated.

galaxyskyknight commented 2 years ago

I found the LEDE repository and it appears to be almost identical to the OpenWRT repository. Perhaps I am looking at the wrong place, can you point me at the repository that you are compiling from. New applications of this code base should not be using CMake as that is less maintained, less understood, less flexible and most importantly it is deprecated.

On LEDE repo, they changed to use CMake makefile at this commit along with upgrading to use n2n 3.0 code base, just FYI https://github.com/coolsnowwolf/lede/commit/886a9fd3004a5dfb1b97086b7fe3f27fef1a6c20

galaxyskyknight commented 2 years ago

so can we merge it?

hamishcoleman commented 2 years ago

Adding this include to the file you suggest is not the right place. As we cannot reproduce this on our supported platforms, we cannot easily test alternatives either

galaxyskyknight commented 2 years ago

Adding this include to the file you suggest is not the right place. As we cannot reproduce this on our supported platforms, we cannot easily test alternatives either

It doesn't make sense to me. I have checked the source code, the orginal compile error comes from the management.h, but it has a reference to the pthread usage like below, however there is no any where has included the pthread.h.And its recursive included file in management.h is n2n_typedef.h, there is also no anywhere to explicitly include the pthread.h header file, but only a compile macro like this:

#ifdef HAVE_PTHREAD
    pthread_t               id;            /* thread id */
    pthread_mutex_t         access;        /* mutex for shared access */
#endif

so this is the reason that the error: there is a reference defination to pthread_t and its mutex in management.h but not include in the header and also no any recursive included for the pthread.h in n2n_typedef.h

I don't know how it could work through in other platform with no any error, perhaps there is other tricky in makefile but I cannot find. The help I am asking is just the investigation for this...but not the prevarication like the way: we are working all well and we cannot reproduce and your platform is not supported and no way test, so your problem I am sorry ....

hamishcoleman commented 2 years ago

You have to understand that we simply cannot support every single build platform that exists, we are all volunteers and do not have the time to install, configure, build and understand them all.

It is not prevarication to say that we cannot reproduce this on the supported build platform. Nor is it reasonable to randomly add an include with no way to test it. Please feel free to write a patch to add your preferred build system in a fully automated CI script so that a repeatable test case can be easily seen by other people.

galaxyskyknight commented 2 years ago

You have to understand that we simply cannot support every single build platform that exists, we are all volunteers and do not have the time to install, configure, build and understand them all.

It is not prevarication to say that we cannot reproduce this on the supported build platform. Nor is it reasonable to randomly add an include with no way to test it. Please feel free to write a patch to add your preferred build system in a fully automated CI script so that a repeatable test case can be easily seen by other people.

OK, I got, thanks.

hamishcoleman commented 2 years ago

@galaxyskyknight have you closed this because you solved your compile issue? Can you let us know how you completed the issue?

galaxyskyknight commented 2 years ago

@galaxyskyknight have you closed this because you solved your compile issue? Can you let us know how you completed the issue?

No, I think I just explained the reason why there should be a inlcude in n2n_typedef.h there, it is not a different platform dependency but a reasonable fix to me, whatever how it could work through there in the past w/o it included, and you have mentioned you cannot simply support every single build platform and no time to do it, so I think we are done here. And I use my priviate fork to include it and close the issue, that's it.

I don't mean I won't to do more but only talk and request, becoz I am not a skillfull developer and not so professional to be a volunteer who can make sure that not mess you guys' job up.

thanks.