build icon indicating copy to clipboard operation
build copied to clipboard

Should the default for the `threading` feature be `multi`?

Open pdimov opened this issue 6 years ago • 10 comments

It's currently single:

feature.feature threading
    : single multi
    : propagated ;

which seems outdated.

pdimov avatar Oct 05 '18 13:10 pdimov

I don't quite understand where does the default value of threading come from. Under Windows, when I say b2 libs/system/build, I get threading=multi by default. But on Linux, the default seems to be single. Can someone explain?

pdimov avatar Oct 09 '18 14:10 pdimov

AMDG

On 10/09/2018 08:10 AM, Peter Dimov wrote:

I don't quite understand where does the default value of threading come from. Under Windows, when I say b2 libs/system/build, I get threading=multi by default.

It's msvc, not windows: https://github.com/boostorg/build/blob/develop/src/tools/msvc.jam#L1694

But on Linux, the default seems to be single. Can someone explain?

In Christ, Steven Watanabe

swatanabe avatar Oct 09 '18 14:10 swatanabe

Ah so. You're right, it's threading=single for toolset=gcc, and for msvc when runtime-link=static. msvc-8.0 and above no longer has a single-threaded runtime of any kind anyway, so that code is probably "wrong", except not really, because you can compile with threading=single and it doesn't matter, threads are still enabled.

What do you think about changing the default to multi? If not on the Build side as I suggest here, then on the Jamroot side, for Boost only?

For some context, I'm taking another stab at generating CMake config files and for that I'm trying to enlist the install targets generated by boost-install, and to make the install target of a library depend on the install targets of its dependencies.

So when I install log, I get

error: Name clash for '<p/home/travis/.local/lib>libboost_system.a'
error: 
error: Tried to build the target twice, with property sets having 
error: these incompatible properties:
error: 
error:     -  <threading>multi
error:     -  <threading>single

which is - I assume - because Log itself doesn't set multi because it supports an unthreaded mode, but since Thread is a dependency of it, I'm trying to install Thread as well, and System, a dependency of both, is conflicted about its path in life.

It seems to me that by default installing Boost with layout=system should use threading=multi, otherwise things can't work.

pdimov avatar Oct 09 '18 14:10 pdimov

But perhaps the correct place to put that default is in default-build of Jamroot?

pdimov avatar Oct 09 '18 15:10 pdimov

AMDG

On 10/09/2018 09:07 AM, Peter Dimov wrote:

But perhaps the correct place to put that default is in default-build of Jamroot?

This already exists, but only for the top level install/stage targets. Do we want to apply it for everything? The fact that it's applied as part of install is likely a historical artifact from when --build-type=complete meant both single and multi-threaded builds.

In Christ, Steven Watanabe

swatanabe avatar Oct 09 '18 15:10 swatanabe

I can't think of ever wanting a threading=single default build, but there could be scenarios that I'm not aware of.

Re top level targets, that's part of another philosophical discussion about whether we should try to transition to a scheme in which the top-level targets just delegate to the library-level ones and do nothing else. So basically alias install-$lib : libs/$lib/build//install ; and then alias install : install-$(libraries) ; and same for stage.

As part of the CMake thing, I've also introduced a meta-library called headers, for which lib/headers/build//install installs the headers. (Dependency-wise, depending on any header-only library is treated as depending on libs/headers.) Staging it could perhaps do what b2 headers does today, so that that part of Jamroot is delegated to a library, too.

pdimov avatar Oct 09 '18 15:10 pdimov

I deduce from the fact that you haven't accepted my PR against type_erasure that you have reservations against this scheme.

You are probably right, in a sense. This is not in the spirit of Boost.Build. The individual libraries are supposed to just declare the targets, not actually do the work. The install (or package.install) rule is supposed to actually do the work of installing their targets.

That's true in principle, but in practice, libraries routinely declare implicit targets that aren't supposed to be installed, have no way of seeing that this is wrong in that no test of theirs fail, and I haven't quite been able to fit cmake config files into this scheme. Installing a library automatically installs its dependencies, but it doesn't know that it should automatically install the cmake config files of its dependencies. And the fact that the libraries go into lib/ while the cmake configs go into lib/cmake/$libname additionally complicates matters as there seems to be no way of expressing this (or if there is, I don't know of it.)

So it seems to me to just accepting the idea that the libraries need to be individually installable by invoking their install target works better in practice, even though it might be less elegant and less b2-ic. This allows people to test the installation of their library (in .travis.yml and by hand) so that they can see if it doesn't work correctly.

But I don't insist that this is correct. It may well not be. Comments welcome. Deducing an opinion by PR nonacceptance is an approximate procedure and I'm not very good at it.

pdimov avatar Oct 09 '18 21:10 pdimov

AMDG

On 10/09/2018 03:58 PM, Peter Dimov wrote:

I deduce from the fact that you haven't accepted my PR against type_erasure that you have reservations against this scheme.

I intend to accept it. I just haven't gotten around to it yet.

You are probably right, in a sense. This is not in the spirit of Boost.Build. The individual libraries are supposed to just declare the targets, not actually do the work. The install (or package.install) rule is supposed to actually do the work of installing their targets.

??? I don't actually have a problem with using boost-install like this. Explicitly declaring which targets should be installed seems a lot more sane than the current system of guessing. However, if we intend to do this, we should make boost-install set up targets that have exactly the same behavior as the current stage and install targets (i.e. using top-level-target).

That's true in principle, but in practice, libraries routinely declare implicit targets that aren't supposed to be installed, have no way of seeing that this is wrong in that no test of theirs fail, and I haven't quite been able to fit cmake config files into this scheme. Installing a library automatically installs its dependencies, but it doesn't know that it should automatically install the cmake config files of its dependencies. And the fact that the libraries go into lib/ while the cmake configs go into lib/cmake/$libname additionally complicates matters as there seems to be no way of expressing this (or if there is, I don't know of it.)

It probably requires changes to package.install to handle it cleanly.

So it seems to me to just accepting the idea that the libraries need to be individually installable by invoking their install target works better in practice, even though it might be less elegant and less b2-ic. This allows people to test the installation of their library (in .travis.yml and by hand) so that they can see if it doesn't work correctly.

But I don't insist that this is correct. It may well not be. Comments welcome. Deducing an opinion by PR nonacceptance is an approximate procedure and I'm not very good at it.

If I intend to reject a PR, I will say so and explain why and close it.

In Christ, Steven Watanabe

swatanabe avatar Oct 09 '18 22:10 swatanabe

I misunderstood then, sorry and thanks for clarifying.

This is what I'm talking about:

https://github.com/boostorg/boost/tree/feature/cmake-config https://github.com/boostorg/headers https://github.com/boostorg/boost_install

and this is what it's supposed to do:

https://github.com/boostorg/boost_install/blob/master/.travis.yml

The idea is that eventually b2 install will install .cmake configuration files along with the libraries, so that CMake projects will then be able to find_package(boost_lib) and then link to Boost::lib. At the moment the individual install targets work, but the top level one doesn't, yet.

pdimov avatar Oct 09 '18 22:10 pdimov

Thank you for your contributions. Main development of B2 has moved to https://github.com/bfgroup/b2 This issue has been automatically marked as "transition" to indicate the potential for needing transition to the new B2 development project.

stale[bot] avatar Jun 26 '21 03:06 stale[bot]