Fix gitian win32 build #1716

pull luke-jr wants to merge 3 commits into bitcoin:master from luke-jr:bugfix_gitian changing 5 files +53 −71
  1. luke-jr commented at 1:51 AM on August 24, 2012: member
    • The patch created in boost-win32.yml was not indented as gitian requires it to be - no longer applicable with this next change:
    • Move the boost::interprocess workaround to Bitcoin-Qt sources, so it works even with native builds (note that Boost 1.50+ do not have the bug).
    • Adjust the paths gitian-win32.yml looks for Boost to reflect new 1.49 version dependency.
  2. in src/init.cpp:None in 759beedc66 outdated 
       7 | +#if defined(WIN32) && BOOST_VERSION < 105000
   8 | +#define BOOST_INTERPROCESS_HAS_WINDOWS_KERNEL_BOOTTIME
   9 | +#define BOOST_INTERPROCESS_HAS_KERNEL_BOOTTIME
  10 | +#include <boost/interprocess/detail/win32_api.hpp>
  11 | +#endif
  12 | +
    luke-jr commented at 1:53 AM on August 24, 2012:

    I'm not sure if init.cpp needs this, but it does include some boost::interprocess stuff, so I included it just to be safe.

    Diapolo commented at 6:40 AM on August 24, 2012:

    I guess init.cpp does not need this, as we don't use message queues there.

    laanwj commented at 1:10 PM on August 24, 2012:

    It only uses boost::interprocess::file_lock in init.cpp, so it's not needed there.

  3. in src/qt/qtipcserver.cpp:None in ca3efac006 outdated 
       1 | @@ -2,6 +2,13 @@
   2 |  // Distributed under the MIT/X11 software license, see the accompanying
   3 |  // file COPYING or http://www.opensource.org/licenses/mit-license.php.
   4 |  
   5 | +#include <boost/version.hpp>
   6 | +#if defined(WIN32) && BOOST_VERSION == 104900
    laanwj commented at 6:05 AM on August 24, 2012:

    Shouldn't this be <= 104900 ?

    Diapolo commented at 6:42 AM on August 24, 2012:

    The code with 1.48 was also different, so to be safe I think that is correct. Matts code checks for < 1.49 for his compiler warning detection, but for the fix to work it has to be 1.49 (with version above that we could remove all that hacks and checks alltogether).

    laanwj commented at 1:08 PM on August 24, 2012:

    Yeah, that's true.

    Diapolo commented at 7:49 AM on August 25, 2012:

    But if we continue to use the work-around for now, @luke-jr should include a small comment here IMHO.

  4. Diapolo commented at 6:38 AM on August 24, 2012: none

    IMHO it would be nice to only need the monkey-patch code in one place.

  5. laanwj commented at 1:10 PM on August 24, 2012: member

    And if you move all message queue related code to qtipcserver.cpp, this workaround could be limited to one file, which'd be nice.

  6. Abstract all IPC communication to qtipcserver bc8d832335
  7. luke-jr commented at 5:01 PM on August 24, 2012: member

    How's that?

  8. TheBlueMatt commented at 8:28 PM on August 24, 2012: member

    Id highly prefer we just simply upgrade to boost 1.50 or later instead of continuing to use this patch.

  9. luke-jr commented at 8:42 PM on August 24, 2012: member

    @TheBlueMatt No reason the Gitian build can't be updated to use 1.50 too, but it sounds non-trivial, and I think @gavinandresen wants to do 0.7rc1 today.

  10. laanwj commented at 7:08 AM on August 25, 2012: member

    I do like the de-duplication of code, even if we're going to remove the workaround.

    Also, note that at least boost 1.51 has a new issue (#1719) which surfaces in Wine but may possibly trigger in other cases when NtQuerySemaphore fails. Be careful.

  11. in src/qt/qtipcserver.cpp:None in ec6493e12c outdated 
       1 | @@ -2,11 +2,19 @@
   2 |  // Distributed under the MIT/X11 software license, see the accompanying
   3 |  // file COPYING or http://www.opensource.org/licenses/mit-license.php.
   4 |  
   5 | +#include <boost/version.hpp>
   6 | +#if defined(WIN32) && BOOST_VERSION == 104900
   7 | +#define BOOST_INTERPROCESS_HAS_WINDOWS_KERNEL_BOOTTIME
   8 | +#define BOOST_INTERPROCESS_HAS_KERNEL_BOOTTIME
   9 | +#include <boost/interprocess/detail/win32_api.hpp>
    Diapolo commented at 7:47 AM on August 25, 2012:

    I'm rather sure you can remove the above include, as win32_api.hpp get's included below message_queue.hpp.

    interprocess/ipc/message_queue.hpp includes: interprocess/detail/managed_open_or_create_impl.hpp includes: interprocess/detail/os_thread_functions.hpp contains:

    #if (defined BOOST_INTERPROCESS_WINDOWS) # include <boost/interprocess/detail/win32_api.hpp>

  12. BitcoinPullTester commented at 2:40 AM on August 26, 2012: none

    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/ec6493e12c1c695cccfb69f31e2f5aaca6f95154 for binaries and test log.

  13. sipa commented at 12:30 AM on August 27, 2012: member

    Can this be merged, or are there still issues? I'd be nice to do Windows gitian builds again...

  14. TheBlueMatt commented at 12:48 AM on August 27, 2012: member

    ACK all except "Bugfix: Move boost::interprocess Win32 workaround to Bitcoin-Qt sourc… ", Id really rather just switch to boost 1.50, I haven't heard anything about bugs as a result, plus we haven't started rc yet...

  15. Bugfix: Move boost::interprocess Win32 workaround to Bitcoin-Qt sources, rather than hacking boost in gitian 1837696580
  16. Bugfix: Adjust gitian-win32 to use Boost 1.49 for real d0377a70e2
  17. luke-jr commented at 12:50 AM on August 27, 2012: member

    Adjusted for @Diapolo's comment. Whether we switch to boost 1.50 or not is another issue IMO, and the 1.49 build should still be fixed either way...

  18. laanwj commented at 2:15 AM on August 27, 2012: member

    I think this should be merged nevertheless. If anything, it is harmless when 1.50+ is used because of the version check.

    Edit: btw @luke-jr, now that you're changing the boost build,

    • can you add runtime-link=static to build only the mt-s variant that we use, instead of mt-s and mt? this just saves some compilation time.
    • also we need a <ranlib>i686-w64-mingw32-ranlib in user-config.jam so that the manual ar xfs (in both boost-win32.xml and gitian-win32.xml) can be skipped.
  19. TheBlueMatt commented at 2:53 AM on August 27, 2012: member

    My point isnt that this shouldn't be merged (it should), but that I'd rather see 1.50 in 0.7 than use any of these patches or odd headers in 0.7, ie dont merge this and do 0.7rc1 right away.

  20. laanwj commented at 3:04 AM on August 27, 2012: member

    BTW I just verified: boost 1.50 does not suffer from #1719.

  21. Diapolo commented at 5:34 AM on August 27, 2012: none

    I have to admit, I really like the idea of directly switching to Boost 1.50 and use that. @laanwj Did you yet report that Boost 1.51 bug to the Boost devs? Would be nice to notify them. Just for comparison, my Boost command-line to build contains --build-type=minimal stage link=static runtime-link=static threading=multi variant=release -a -j 4 --with-filesystem --with-program_options --with-system --with-thread

  22. gavinandresen commented at 3:57 PM on August 27, 2012: contributor

    Compiling on the mac, I get warning: src/qt/qtipcserver.cpp:29: warning: ‘void ipcThread2(void*)’ declared ‘static’ but never defined

  23. Diapolo commented at 4:34 PM on August 27, 2012: none

    Well ipcThread2() is currently unused on Mac, perhaps we should put the declaration into an else-branch here: https://github.com/bitcoin/bitcoin/pull/1716/files#L3R37?

  24. gavinandresen merged this on Aug 27, 2012
  25. gavinandresen closed this on Aug 27, 2012
  26. DrahtBot locked this on Sep 8, 2021
Contributors
luke-jrDiapololaanwjTheBlueMattBitcoinPullTestersipagavinandresen
Linked (view graph)
#32089 Failure to run Fuzz tests when running with corpus
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: 2026-04-14 15:16 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me