Get boost.thread building

[andrewkatson]

boost.thread builds now. The test directory has no CMakeLists.txt file so I don't know how to setup the tests so I didn't bother. Also the top level .txt file says this should be fine since it has an if clause looking for the subdirectory .txt file and does nothing with tests if it does not exist.

[zaucy]

the tests might not have a cmake file, but there's a jamfile. Even just having 1 test running would be really helpful. Doesn't have to be all of them.

[zaucy]

I can merge without the test though. I'll probably add it after when I do some testing.

[andrewkatson]

Oh Ill add tests. I wasn't sure if I should since there was nothing to go on but I can do it. So hold off on merging.

[zaucy]

Thanks! Make sure there's a separate module for the test. Its a great way to test how people will experience the module downstream and useful for separating test dependencies.

[andrewkatson]

Gotcha. I have the tests all rounded up but the build is failing because boost.system is missing .ipp files so I added a PR for that: bazelboost/system#1

[zaucy]

merged! should be available in the registry as version 1.83.0.bzl.2 now 👍

[zaucy]

btw even if boost.system wasn't released you could test it with git_override to get the CI passing.

[andrewkatson]

I added tests and it now requires that boost.chrono have srcs in it. I also excluded various tests that had weird things like missing headers, or deprecated header files, or assigning non const lvalues to string or missing bindings etc... I figured that was out of the scope of this change. Is it fine to push now?

[zaucy]

Need a test module so the test dependencies don't get added to the main module. Please add test/MODULE.bazel.

[zaucy]

added some github actions that I have on other boost module repo

[andrewkatson]

Ok I got it to this point. Chrono needs to be updated. Do you want me to do that first then this one?

ERROR: /Users/andrewkatson/thread/test/BUILD.bazel:3:8: Linking tests failed: (Exit 1): cc_wrapper.sh failed: error executing CppLink command (from target //:tests) external/bazel_tools~cc_configure_extension~local_config_cc/cc_wrapper.sh @bazel-out/darwin_arm64-fastbuild/bin/tests-2.params

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
Undefined symbols for architecture arm64:
  "boost::chrono::steady_clock::now()", referenced from:
      bool boost::condition_variable::wait_for<long long, boost::ratio<1l, 1000l>, bool (*)()>(boost::unique_lock<boost::mutex>&, boost::chrono::duration<long long, boost::ratio<1l, 1000l>> const&, bool (*)()) in test_11796.o
      bool boost::condition_variable::wait_until<boost::chrono::steady_clock, boost::chrono::duration<long long, boost::ratio<1l, 1000000000l>>, bool (*)()>(boost::unique_lock<boost::mutex>&, boost::chrono::time_point<boost::chrono::steady_clock, boost::chrono::duration<long long, boost::ratio<1l, 1000000000l>>> const&, bool (*)()) in test_11796.o
      bool boost::condition_variable::wait_for<long long, boost::ratio<1l, 1l>, bool (*)()>(boost::unique_lock<boost::mutex>&, boost::chrono::duration<long long, boost::ratio<1l, 1l>> const&, bool (*)()) in test_12293.o
      bool boost::condition_variable::wait_for<int, boost::ratio<3600l, 1l>, bool (*)()>(boost::unique_lock<boost::mutex>&, boost::chrono::duration<int, boost::ratio<3600l, 1l>> const&, bool (*)()) in test_366_1.o
      boost::cv_status boost::condition_variable::wait_for<long long, boost::ratio<1l, 1l>>(boost::unique_lock<boost::mutex>&, boost::chrono::duration<long long, boost::ratio<1l, 1l>> const&) in test_8960.o
      boost::cv_status boost::condition_variable::wait_until<boost::chrono::steady_clock, boost::chrono::duration<long long, boost::ratio<1l, 1000000000l>>>(boost::unique_lock<boost::mutex>&, boost::chrono::time_point<boost::chrono::steady_clock, boost::chrono::duration<long long, boost::ratio<1l, 1000000000l>>> const&) in test_8960.o
      boost::cv_status boost::condition_variable::wait_until<boost::chrono::steady_clock, boost::chrono::duration<long long, boost::ratio<1l, 1000000000l>>>(boost::unique_lock<boost::mutex>&, boost::chrono::time_point<boost::chrono::steady_clock, boost::chrono::duration<long long, boost::ratio<1l, 1000000000l>>> const&) in test_8960.o
      ...
  "boost::chrono::system_clock::now()", referenced from:
      thread() in test_4882.o
      boost::cv_status boost::condition_variable::wait_until<boost::chrono::steady_clock, boost::chrono::duration<long long, boost::ratio<1l, 1000000000l>>>(boost::unique_lock<boost::mutex>&, boost::chrono::time_point<boost::chrono::steady_clock, boost::chrono::duration<long long, boost::ratio<1l, 1000000000l>>> const&) in test_8960.o
ld: symbol(s) not found for architecture arm64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
Target //:tests failed to build
Use --verbose_failures to see the command lines of failed build steps.
INFO: Elapsed time: 30.010s, Critical Path: 3.45s
INFO: 125 processes: 11 internal, 114 darwin-sandbox.
ERROR: Build did NOT complete successfully
//:tests   

[zaucy]

if boost.thread requires boost.chrono then doing that one first is probably a good idea. If it's optional though, then we could configure it to not use it (or exclude the tests the require it). Either way we should make sure the CI is passing before merging.

[andrewkatson]

Okay Ill go do chrono and get back to this later!

[andrewkatson]

Hey quick question. Each boost test defines its own main method so I cannot glob them into one cc_test rule. What is the solution here? Define a cc_test per test file? Write my own test main method and remove all the others? Or is there some bazel magic?

[andrewkatson]

Okay so sad news. I tried and did --keep_going and every single test fails to build if you exclude the one file (pthread_helpers.hpp) from the build of @boost.thread. So until that file gets fixed (with the assert import and other changes probably) then we cannot get any tests to run on Posix. Unless there is a way in bazel to get this to work? I was thinking maybe generate a file or progrmatically modify `pthread_helpers.hpp"

[zaucy]

wow yeah that is a real bummer. I'm not sure how they're getting that to work 🤔

[zaucy]

@andrewkatson it might be this:

thread/test/Jamfile.v2

Lines 279 to 280 in f339fa2

	all_rules += [ compile self_contained_header.cpp : <define>"BOOST_THREAD_TEST_HEADER=$(rel_file)" <dependency>$(file) : $(test_name) ] ;
	all_rules += [ compile self_contained_header.cpp : <define>"BOOST_THREAD_TEST_HEADER=$(rel_file)" <define>"BOOST_THREAD_TEST_POST_WINDOWS_H" <dependency>$(file) <conditional>@windows-cygwin-specific : $(test_name)-post_winh ] ;

defining BOOST_THREAD_TEST_HEADER with something might help.

[andrewkatson]

@andrewkatson it might be this:

thread/test/Jamfile.v2

Lines 279 to 280 in f339fa2

	all_rules += [ compile self_contained_header.cpp : <define>"BOOST_THREAD_TEST_HEADER=$(rel_file)" <dependency>$(file) : $(test_name) ] ;
	all_rules += [ compile self_contained_header.cpp : <define>"BOOST_THREAD_TEST_HEADER=$(rel_file)" <define>"BOOST_THREAD_TEST_POST_WINDOWS_H" <dependency>$(file) <conditional>@windows-cygwin-specific : $(test_name)-post_winh ] ;

defining BOOST_THREAD_TEST_HEADER with something might help.

I have been trying for the last 2 hours to in place modify a file with bazel genrules and/or custom rules and as far as I can tell it cannot be done. There is a hard limit on making a src == to an output and there is a cycle created if you do something like move a fileA > fileB then modify fileB and move it back to where fileA was with two or more genrules. So even if I generate a new boost header file I would need to include it in existing files afaik I cannot do that. The only other option I see is to modify a series of files so that all the includes are changed. So instead of pthread_helpers.hpp you would have helpers.hpp and you would need to replace that across files and then replace those files everywhere and it gets really brittle. LMK if you have any other ideas because I am scratching my head. I get why bazel doesn't allow the in place modification of files with genrules but it is an annoying limitation.

[andrewkatson]

Okay I managed to get bazel to answer and they said to ask boost to modify the files if we want things to work. How realistic is that?

[andrewkatson]

Outside of adding #include <boost/assert.hpp> to pthread_helpers.hpp everything else will work now with my latest commit. and we get to keep ~80 tests.

[andrewkatson]

@zaucy I tried defining BOOST_THREAD_TEST_HEADER and that did not fix things. What should we do?

[zaucy]

I'll be able to look into this in the next few days. Will post back then!

[andrewkatson]

No problem! :) I get so eager

[andrewkatson]

Hey any update on what we do with this?

[zaucy]

Ill take a look today! Thanks for the reminder.

[zaucy]

I think we have no choice but to modify the the source. I added a PR upstream to make the change more official though boostorg#401

[andrewkatson]

Thank you for making the change to boost! Yeah its an odd situation but at least its only an added header not major structural changes.

[zaucy]

@andrewkatson should be available as version 1.83.0.bzl.2 in the registry! Thanks for all your hard work and patience.

[andrewkatson]

AWESOME! thank you!!