Skip to content

Conversation

@jjm2473
Copy link
Contributor

@jjm2473 jjm2473 commented Jun 10, 2025

-isystem ext makes #include <miniupnpc/miniupnpc.h> actually include ext/miniupnpc/miniupnpc.h.

we should use $(STAGING_DIR)/usr/include as a higher priority system include path.

Fix openwrt/openwrt#18019

📦 Package Details

Maintainer: @
(You can find this by checking the history of the package Makefile.)

Description:
Fix openwrt/openwrt#18019


🧪 Run Testing Details

  • **OpenWrt Version: 24.10.1
  • **OpenWrt Target/Subtarget: armsr/armv8
  • **OpenWrt Device: virtualbox

✅ Formalities

  • I have reviewed the CONTRIBUTING.md file for detailed contributing guidelines.

If your PR contains a patch:

  • It can be applied using git am
  • It has been refreshed to avoid offsets, fuzzes, etc., using
    make package/<your-package>/refresh V=s
  • It is structured in a way that it is potentially upstreamable
    cannot be upstram, this is a part of openwrt cross compiling path hack.

@1715173329
Copy link
Member

Cc maintainer @mwarning

@xuanranran
Copy link
Contributor

This patch also fixes the issue with running ZeroTier on x86_64 firmware compiled with gcc14. Kudos to the author for providing the patch.

@qingtian110
Copy link

This method is proven to be effective istoreos/istoreos#2358 (comment)

@BKPepe
Copy link
Member

BKPepe commented Jun 10, 2025

This method is proven to be effective istoreos/istoreos#2358 (comment)

Yeah, it is being discussed in zerotier/ZeroTierOne#2453

and I see that @mwarning created this PR and it was merged: #26088 and it was also backported to OpenWrt 24.10 (#26323).

@jjm2473
Copy link
Contributor Author

jjm2473 commented Jun 10, 2025

and I see that @mwarning created this PR and it was merged: #26088 and it was also backported to OpenWrt 24.10 (#26323).

@BKPepe

#26088 does not completly fix the issue openwrt/openwrt#18019, that's why it was reopened. Because zerotier still use it's own miniupnpc header, so MINIUPNPC_API_VERSION < 18 still true, the patched code did not take effect.

@jjm2473
Copy link
Contributor Author

jjm2473 commented Jun 10, 2025

This method is proven to be effective istoreos/istoreos#2358 (comment)

Yes, this patch should also work, but it changes more .c .h source, this could easily lead to conflicts in the future.

@jjm2473 jjm2473 marked this pull request as draft June 10, 2025 13:32
@jjm2473
Copy link
Contributor Author

jjm2473 commented Jun 10, 2025

@BKPepe I move CFLAGS to OpenWRT Makefile, is it better?

If it's better, I will squash two commits.

@jjm2473 jjm2473 marked this pull request as ready for review June 10, 2025 13:57
@Neustradamus
Copy link

@jjm2473: Thanks for your PR!

@jjm2473 jjm2473 force-pushed the zerotier-fix-miniupnp-path branch from d67aac7 to e8de6d4 Compare June 11, 2025 02:25
@1715173329
Copy link
Member

The latter implementation looks better to me.

@1715173329
Copy link
Member

btw please bump PKG_RELEASE

@jjm2473 jjm2473 force-pushed the zerotier-fix-miniupnp-path branch from e8de6d4 to c87f6f0 Compare June 11, 2025 05:18
@BKPepe
Copy link
Member

BKPepe commented Jun 11, 2025

The latter implementation looks better to me.

Yeah, I agree this is much better. 💯 But the issue still lies within zerotier. They need to handle it.

`-isystem ext` makes `#include <miniupnpc/miniupnpc.h>` actually include `ext/miniupnpc/miniupnpc.h`.

we should use `$(STAGING_DIR)/usr/include` as a higher priority system include path.

Fix openwrt/openwrt#18019

Signed-off-by: Liangbin Lian <jjm2473@gmail.com>
@jjm2473 jjm2473 force-pushed the zerotier-fix-miniupnp-path branch from c87f6f0 to f4a35ed Compare June 11, 2025 09:20
@jjm2473
Copy link
Contributor Author

jjm2473 commented Jun 11, 2025

Yeah, I agree this is much better. 💯 But the issue still lies within zerotier. They need to handle it.

The main problem is that zerotier does not officially support the use of external libminiupnpc when cross-compiling.

@BKPepe BKPepe merged commit d1b5a6d into openwrt:master Jun 11, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Zerotier segmentation fault on startup.

6 participants