build: miniupnpc 2.2.2 #20421

pull fanquake wants to merge 1 commits into bitcoin:master from fanquake:miniupnpc_220 changing 4 files +37 −33
  1. fanquake commented at 3:11 am on November 19, 2020: member

    Creating the dll subdir is no-longer required. We can drop our wingen patch.

    One issue that came up in #19867:

    unrelated to this change but: why are we inserting the architecture in here, seems like something not necessary to reveal

    My assumption is that it was being inserted to make depends more deterministic. However I think we can improve this, as there’s no reason to reveal more of the version information either. Could leave the version as is /2.0 and either drop the architecture, or insert something else?

    I’ve dropped our sed and added a patch that just removes the OS string and miniupnpc version from the User-Agent. i.e:

    0# master
1strings depends/x86_64-apple-darwin19.6.0/lib/libminiupnpc.a | rg -i User-Agent
2User-Agent: x86_64-apple-darwin19.6.0, UPnP/1.1, MiniUPnPc/2.0.20180203
3User-Agent: x86_64-apple-darwin19.6.0, UPnP/1.1, MiniUPnPc/2.0.20180203
4
5# this PR
6strings depends/x86_64-apple-darwin19.6.0/lib/libminiupnpc.a | rg -i User-Agent
7User-Agent: UPnP/1.1
8User-Agent: UPnP/1.1

    Note that built unmodified (https://github.com/miniupnp/miniupnp/commit/22c13863518adfc4eae11de82cb55a69414afdae), the User-Agent would be:

    0strings libminiupnpc.dylib | rg User-Agent                                     
1User-Agent: Darwin/19.6.0, UPnP/1.1, MiniUPnPc/2.2.0
2User-Agent: Darwin/19.6.0, UPnP/1.1, MiniUPnPc/2.2.0
  2. fanquake added the label Build system on Nov 19, 2020
  3. fanquake added this to the milestone 22.0 on Nov 19, 2020
  4. fanquake commented at 4:55 am on November 19, 2020: member

    This is currently failing on just the macOS cross build because it has -Werror enabled, which means means-Werror=gnu, and -Wzero-length-array is included in -Wgnu:

     0/usr/bin/ccache /tmp/cirrus-ci-build/depends/x86_64-apple-darwin16/native/bin/clang++ -target x86_64-apple-darwin16 -mmacosx-version-min=10.12 --sysroot /tmp/cirrus-ci-build/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers -stdlib=libc++ -mlinker-version=530 -B/tmp/cirrus-ci-build/depends/x86_64-apple-darwin16/native/bin -nostdinc++ -isystem /tmp/cirrus-ci-build/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers/usr/include/c++/v1 -std=c++17 -DHAVE_CONFIG_H -I. -I../src/config   -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -I.  -DBOOST_SP_USE_STD_ATOMIC -DBOOST_AC_USE_STD_ATOMIC -pthread -I/tmp/cirrus-ci-build/depends/x86_64-apple-darwin16/share/../include -I./leveldb/include -I./leveldb/helpers/memenv -I./secp256k1/include -I./univalue/include  -I/tmp/cirrus-ci-build/depends/x86_64-apple-darwin16/include -D_THREAD_SAFE -I/tmp/cirrus-ci-build/depends/x86_64-apple-darwin16/include -I/tmp/cirrus-ci-build/depends/x86_64-apple-darwin16/share/../include/  -DHAVE_BUILD_INFO -D__STDC_FORMAT_MACROS -DMAC_OSX -DOBJC_OLD_DISPATCH_PROTOTYPES=0  -Wstack-protector -fstack-protector-all -fcf-protection=full   -Werror=gnu -Werror=vla -Werror=shadow-field -Werror=switch -Werror=thread-safety -Werror=range-loop-analysis -Werror=unused-variable -Werror=date-time -Werror=return-type -Werror=conditional-uninitialized -Werror=sign-compare -Werror=unreachable-code-loop-increment    -pipe -O2  -fvisibility=hidden -c -o interfaces/libbitcoin_server_a-chain.o `test -f 'interfaces/chain.cpp' || echo './'`interfaces/chain.cpp
 1/usr/bin/ccache /tmp/cirrus-ci-build/depends/x86_64-apple-darwin16/native/bin/clang++ -target x86_64-apple-darwin16 -mmacosx-version-min=10.12 --sysroot /tmp/cirrus-ci-build/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers -stdlib=libc++ -mlinker-version=530 -B/tmp/cirrus-ci-build/depends/x86_64-apple-darwin16/native/bin -nostdinc++ -isystem /tmp/cirrus-ci-build/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers/usr/include/c++/v1 -std=c++17 -DHAVE_CONFIG_H -I. -I../src/config   -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -I.  -DBOOST_SP_USE_STD_ATOMIC -DBOOST_AC_USE_STD_ATOMIC -pthread -I/tmp/cirrus-ci-build/depends/x86_64-apple-darwin16/share/../include -I./leveldb/include -I./leveldb/helpers/memenv -I./secp256k1/include -I./univalue/include  -I/tmp/cirrus-ci-build/depends/x86_64-apple-darwin16/include -D_THREAD_SAFE -I/tmp/cirrus-ci-build/depends/x86_64-apple-darwin16/include -I/tmp/cirrus-ci-build/depends/x86_64-apple-darwin16/share/../include/  -DHAVE_BUILD_INFO -D__STDC_FORMAT_MACROS -DMAC_OSX -DOBJC_OLD_DISPATCH_PROTOTYPES=0  -Wstack-protector -fstack-protector-all -fcf-protection=full   -Werror=gnu -Werror=vla -Werror=shadow-field -Werror=switch -Werror=thread-safety -Werror=range-loop-analysis -Werror=unused-variable -Werror=date-time -Werror=return-type -Werror=conditional-uninitialized -Werror=sign-compare -Werror=unreachable-code-loop-increment    -pipe -O2  -fvisibility=hidden -c -o interfaces/libbitcoin_server_a-node.o `test -f 'interfaces/node.cpp' || echo './'`interfaces/node.cpp
 2/usr/bin/ccache /tmp/cirrus-ci-build/depends/x86_64-apple-darwin16/native/bin/clang++ -target x86_64-apple-darwin16 -mmacosx-version-min=10.12 --sysroot /tmp/cirrus-ci-build/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers -stdlib=libc++ -mlinker-version=530 -B/tmp/cirrus-ci-build/depends/x86_64-apple-darwin16/native/bin -nostdinc++ -isystem /tmp/cirrus-ci-build/depends/SDKs/Xcode-11.3.1-11C505-extracted-SDK-with-libcxx-headers/usr/include/c++/v1 -std=c++17 -DHAVE_CONFIG_H -I. -I../src/config   -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -I.  -DBOOST_SP_USE_STD_ATOMIC -DBOOST_AC_USE_STD_ATOMIC -pthread -I/tmp/cirrus-ci-build/depends/x86_64-apple-darwin16/share/../include -I./leveldb/include -I./leveldb/helpers/memenv -I./secp256k1/include -I./univalue/include  -I/tmp/cirrus-ci-build/depends/x86_64-apple-darwin16/include -D_THREAD_SAFE -I/tmp/cirrus-ci-build/depends/x86_64-apple-darwin16/include -I/tmp/cirrus-ci-build/depends/x86_64-apple-darwin16/share/../include/  -DHAVE_BUILD_INFO -D__STDC_FORMAT_MACROS -DMAC_OSX -DOBJC_OLD_DISPATCH_PROTOTYPES=0  -Wstack-protector -fstack-protector-all -fcf-protection=full   -Werror=gnu -Werror=vla -Werror=shadow-field -Werror=switch -Werror=thread-safety -Werror=range-loop-analysis -Werror=unused-variable -Werror=date-time -Werror=return-type -Werror=conditional-uninitialized -Werror=sign-compare -Werror=unreachable-code-loop-increment    -pipe -O2  -fvisibility=hidden -c -o libbitcoin_server_a-net.o `test -f 'net.cpp' || echo './'`net.cpp
 3In file included from net.cpp:36:
 4In file included from /tmp/cirrus-ci-build/depends/x86_64-apple-darwin16/share/../include/miniupnpc/miniupnpc.h:14:
 5/tmp/cirrus-ci-build/depends/x86_64-apple-darwin16/share/../include/miniupnpc/upnpdev.h:27:14: error: zero size arrays are an extension [-Werror,-Wzero-length-array]
 6        char buffer[0];
 7                    ^
 81 error generated.
 9Makefile:11312: recipe for target 'libbitcoin_server_a-net.o' failed
10make[2]: *** [libbitcoin_server_a-net.o] Error 1

    Note that the ARM CI, which also has -Werror, and uses depends, is not currently failing. I assume because __GNUC__ is not defined. The problem code:

    0#if defined(__STDC_VERSION) && __STDC_VERSION__ >= 199901L
1	/* C99 flexible array member */
2	char buffer[];
3#elif defined(__GNUC__)
4	char buffer[0];
5#else
6	/* Fallback to a hack */
7	char buffer[1];
8#endif

    was introduced in miniupnpc in https://github.com/miniupnp/miniupnp/commit/47a55b27c7d6232388505cb6a2dec04c3f2123dc.

  5. in depends/patches/miniupnpc/dont_leak_info.patch:29 in 4cc41b389c outdated 
    24+@@ -444,7 +444,7 @@ miniwget3(const char * host,
25+                  "GET %s HTTP/%s\r\n"
26+ 			     "Host: %s:%d\r\n"
27+ 				 "Connection: Close\r\n"
28+-				 "User-Agent: " OS_STRING ", " UPNP_VERSION_STRING ", MiniUPnPc/" MINIUPNPC_VERSION_STRING "\r\n"
29++				 "User-Agent: " UPNP_VERSION_STRING "\r\n"
    laanwj commented at 9:28 am on November 19, 2020:
    Nice, thanks for doing this.
    luke-jr commented at 11:55 pm on November 24, 2020:
    Can we upstream this as a public API option?
  6. hebasto commented at 9:45 am on November 19, 2020: member
    Concept ACK.
  7. practicalswift commented at 10:54 am on November 19, 2020: contributor

    Concept ACK

    Thanks for taking care of the version leaking. I’ve never understood why miniupnpc choose to leak verbose version info like this: it reduces the cost of successful attack a lot.

  8. laanwj commented at 2:54 pm on November 23, 2020: member

    The problem is that there is no legal way to do this in C++ at all, besides through the GNU extension. The [1] hack is all you can get away with in any standard, and it might run into issues with static and dynamic checkers.

    If you want that, you can apparently stop clang from defining __GNUC__ with -fgnuc-version=0. That said, passing -Wno-zero-length-array might be more reasonable.

    Note that the ARM CI, which also has -Werror, and uses depends, is not currently failing. I assume because GNUC is not defined.

    I suppose that’s because the ARM builds use gcc, not clang, which doesn’t have that warning.

  9. laanwj added the label Upstream on Nov 23, 2020
  10. luke-jr commented at 11:57 pm on November 24, 2020: member
    Concept ACK
  11. hebasto commented at 4:53 pm on March 22, 2021: member

    Approach ACK 4cc41b389cf7d7e2a181146871b2dd5e2e05f374.

    I think CI will pass after #20182 (https://cirrus-ci.com/build/6495395879583744).

    Maybe rebase? And, probably, bump version to 2.2.2?

  12. build: miniupnpc 2.2.2 
    Creating the dll subdir is no-longer required.
We can also drop our wingen patch.
    180dc3c886
  13. fanquake commented at 0:49 am on March 23, 2021: member

    I think CI will pass after #20182 (https://cirrus-ci.com/build/6495395879583744).

    I’m not sure why that would make a difference? The same code causing the -Wzero-length-array issue is still in the upnpdev.h header, and we’re still building with --enable-werror:

    0In file included from mapport.cpp:25:
1In file included from bitcoin/depends/x86_64-apple-darwin19.6.0/include/miniupnpc/miniupnpc.h:14:
2bitcoin/depends/x86_64-apple-darwin19.6.0/include/miniupnpc/upnpdev.h:27:14: error: zero size arrays are an extension [-Werror,-Wzero-length-array]
3        char buffer[0];
4                    ^
51 error generated.

    In any case, I’ve rebased, and updated this branch to version 2.2.2. Will just look at patching out the problematic code.

  14. fanquake renamed this:
    build: miniupnpc 2.2.0
    build: miniupnpc 2.2.2
    on Mar 23, 2021
  15. fanquake force-pushed on Mar 23, 2021
  16. hebasto commented at 2:06 am on March 23, 2021: member

    … and we’re still building with --enable-werror

    Right. Plus --enable-suppress-external-warnings :)

  17. fanquake commented at 2:08 am on March 23, 2021: member

    Right. Plus –enable-suppress-external-warnings :)

    Sure, in the CI. However that’s not useful for regular users who just want to build with --enable-werror. Seems like you’re suggesting that --enable-werror is going to become useless without --enable-suppress-external-warnings? That’s not really a path I want to go down. I don’t like the idea of just suppressing everything for the sake of a warning-less build.

  18. hebasto commented at 2:10 am on March 23, 2021: member

    Right. Plus –enable-suppress-external-warnings :)

    Sure, in the CI. However that’s not useful for regular users who just want to build with --enable-werror. Seems like you’re suggesting that --enable-werror is going to become useless without --enable-suppress-external-warnings?

    That was the point to introduce the latter, no?

  19. hebasto approved
  20. hebasto commented at 4:21 am on March 23, 2021: member

    ACK 180dc3c8863fc42e93657eda839001a2a35ef634.

    Gitian builds:

    • Linux:
     0Generating report
 1ceecd19802fe907d38a2ddda188feb42b5cb6c0c928242481beb8540f83c6028  bitcoin-180dc3c8863f-aarch64-linux-gnu-debug.tar.gz
 24e8bab9604433f104ac28db02be353fe14665abbe38056950022fbbdd4a99148  bitcoin-180dc3c8863f-aarch64-linux-gnu.tar.gz
 31ccf71e01c5c547d4f1e104735814851ab7e6c0eddf83b56f287a7269641a561  bitcoin-180dc3c8863f-arm-linux-gnueabihf-debug.tar.gz
 4c817879efad5439806207f69b189f41025a3a2ee85f24d5b561836252d555ef8  bitcoin-180dc3c8863f-arm-linux-gnueabihf.tar.gz
 5ac233f5ad64369ccbdad2bd7ab19d0510f1a2fa1a6e2e4cd1adab8c6ad94fd8a  bitcoin-180dc3c8863f-powerpc64-linux-gnu-debug.tar.gz
 678884c8e636eeacb810912d1ab801cc061874641c388b6d561038f7d9dd05a34  bitcoin-180dc3c8863f-powerpc64-linux-gnu.tar.gz
 71e159cb35511dbc937d7436bea68ab068f9bad80b44bb627d1b80e235f00f886  bitcoin-180dc3c8863f-powerpc64le-linux-gnu-debug.tar.gz
 8f1dca9e18336039384d944ca9c151c417e13b5faa9bb677d2005a7a959114319  bitcoin-180dc3c8863f-powerpc64le-linux-gnu.tar.gz
 949dd9e7be46c15a00400bad53be3345756f1cba04879c63184bcea0a92d8c5ae  bitcoin-180dc3c8863f-riscv64-linux-gnu-debug.tar.gz
1042b23d405d0badb18aa995b150b54e42d3511ea3f73f660734fcab99ba11e16e  bitcoin-180dc3c8863f-riscv64-linux-gnu.tar.gz
11cb7f6547050629ade7afb73c50cd069819f110adbb52cb0ba1ca38bde266d4d1  bitcoin-180dc3c8863f-x86_64-linux-gnu-debug.tar.gz
12a4266cf56175445544ec10c2349e2761676d326900d0674b5d3ffb782388be55  bitcoin-180dc3c8863f-x86_64-linux-gnu.tar.gz
139160e96fcd64634d4f33885241b6ab4b7884a4504431777b790b67e74e6fdc69  src/bitcoin-180dc3c8863f.tar.gz
143e9532e5fcb454a2356eadef4b7d0b32828df8ea29290d1cff1154e9b547b818  bitcoin-core-linux-22-res.yml
15Done.

    bitcoin-180dc3c8863f-x86_64-linux-gnu.tar.gz tested on Linux Mint 20.1

    • Windows:
    0Generating report
10df1d25cd591b12512faefbff9e5be2b51ce980e04ddd51b83cc35449b917fde  bitcoin-180dc3c8863f-win-unsigned.tar.gz
22ad623c3dad5e1f24b8e672bec97f3cb21e8bf0aa768f4be2762486f84cab064  bitcoin-180dc3c8863f-win64-debug.zip
3f5e34a3ca32756303f2303a2153b999f7f51e8ea01e1713e4e1c8553b72c2aea  bitcoin-180dc3c8863f-win64-setup-unsigned.exe
44c1b4313875269aee017e0cc985b3bef8e880537d5fe80b0b858ffe85ca95a76  bitcoin-180dc3c8863f-win64.zip
59160e96fcd64634d4f33885241b6ab4b7884a4504431777b790b67e74e6fdc69  src/bitcoin-180dc3c8863f.tar.gz
6115ea6a8a906ec25a8f1d63bfe41368e5ebaceb68802aa1ba053c179495181c5  bitcoin-core-win-22-res.yml
7Done.

    bitcoin-180dc3c8863f-win64.zip tested on Windows 10 Pro 20H2 (build 19042.867)

    • macOS:
    0Generating report
1f7825c2827390ebda9fd8853f90374927219924a2ef916f2d9ca6e219f2db3ab  bitcoin-180dc3c8863f-osx-unsigned.dmg
2a88d9a0c95b87cdc3f07ffd85c2d3563333c8042218ef2a2fb467494dcf177de  bitcoin-180dc3c8863f-osx-unsigned.tar.gz
316f58d69cb717434fda1cae05a8f1dbc9b63aaff32f197bf7b1d418a267a1140  bitcoin-180dc3c8863f-osx64.tar.gz
49160e96fcd64634d4f33885241b6ab4b7884a4504431777b790b67e74e6fdc69  src/bitcoin-180dc3c8863f.tar.gz
5fa1bed6220152c9cf5b67f4b84b09245dd0818cc80bfffd10c8fc50c554a93ea  bitcoin-core-osx-22-res.yml
6Done.

    bitcoin-180dc3c8863f-osx-unsigned.dmg tested on macOS BigSur 11.2.3 (20D91)

    A router powered by OpenWrt was used for testing UPnP functionality.

  21. laanwj commented at 7:34 pm on March 23, 2021: member
    I’ve done a gitian build and my hashes exactly match @hebasto’s Code review ACK 180dc3c8863fc42e93657eda839001a2a35ef634
  22. laanwj merged this on Mar 23, 2021
  23. laanwj closed this on Mar 23, 2021
  24. sidhujag referenced this in commit e17ca02bea on Mar 23, 2021
  25. random-zebra referenced this in commit 470f9555cc on Apr 30, 2021
  26. fanquake deleted the branch on May 8, 2021
  27. barton2526 referenced this in commit 71dac558fd on Jun 12, 2021
  28. DrahtBot locked this on Aug 16, 2022


fanquake laanwj luke-jr hebasto practicalswift

Labels
Build system Upstream

Milestone
22.0

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-21 15:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me