external: update miniupnpc 2.2.8 #9367
Conversation
1. LINKS flag, will link publicly the libraries provided. 2. PRIVATE_LINKS flag, will link privately the libraries provided. 3. INCLUDES flag, will includes publicly the dirs provided. 4. PRIVATE_INCLUDES flag, will include privately the libraries provided.
|
@0xFFFC0000 any update on this? |
It is ready, has been since its release. Needs reviews. |
| @@ -35,8 +35,13 @@ | |||
| # ...except for FreeBSD, because FreeBSD is a special case that doesn't play well with | |||
| # others. | |||
|
|
|||
| if(NOT MSVC) | |||
| add_compile_options(-D_GNU_SOURCE) | |||
There was a problem hiding this comment.
Is this needed since we add _GNU_SOURCE to compile options in the root CMakeLists.txt?
There was a problem hiding this comment.
Yes, since we add it to CMAKE_C_FLAGS [1] and it does not pass it to projects added via add_subdirectory. It fixes this build issue [2].
|
@selsta i notice that wownero merged this https://codeberg.org/wownero/wownero/commit/5fe3cf2349ae277f1c606332eb16916a640e1e48 maybe broken there too. Didnt check with them. Just thought i'd mention |
|
@nahuhh wowario reported it to me :) |
|
The problem is they have enforced 3.14 here [1]. Give me a little bit of time to think about this. It is dilemma, they are strictly requiring 3.14, and at the same time we don't want 3.14. https://github.com/miniupnp/miniupnp/blob/master/miniupnpc/CMakeLists.txt#L1 |
|
Guix has CMake 3.24.2, however considering that miniupnpc is packaged everywhere, and we don't apply any patches, I don't see why we would need to keep this dependency as a submodule. I would prefer moving miniupnpc to depends (#9273) for sake of consistency. This approach also speeds up recursive checkouts and saves package maintainers the effort of unvendoring this library. I would also be in favor of simply removing miniupnpc since it has a long history of vulnerabilities and isn't very actively maintained. |
Fix and improve existing CMake script.
Fixes: #9359
Related: #9366