Auto-detection of <threadapi> based on <target-os>

[karzhenkov]

Currently Boost.Thread selects the default <threadapi> value based on host system name. How to make it dependent of <target-os> was discussed in tickets #3393 and #7706. My solution is somewhat similar to this proposal but also allows <threadapi> to be specified in build request and improves the <tag> feature behavior:

• <threadapi> has an additional value native which is the default. If build request doesn't specify other value, it is replaced either with "pthread" or "win32" depending on <target-os>.
• <tag> modifies name of generated library only if resulting <threadapi> value differs from one that would have been chosen to replace native according to <target-os>.

[viboes]

Hi,

I'm not an expert on Bjam.

AFAIK, the default is already the native, isn't it?

[karzhenkov]

Currently the default value for <threadapi> is selected as follows:

local api = pthread ; if [ os.name ] = "NT" { api = win32 ; }

Here os.name is name of the host system. The problem arises when target system differs from the host.

I'm trying to cross-compile for QNX and my native API is pthread. But I do this on Windows, and API defaults to win32.

Even if I explicitly specify threadapi in the build request, there is an issue with the tag. My library gets named libboost_thread_pthread.so instead of expected libboost_thread.so.

[viboes]

I don't understand how that tag is used. Could you help me? What should be the name of the library depending on and ?

[viboes]

Hi,

in https://ci.appveyor.com/project/viboes/thread/build/1.0.57-develop we see some issues with native_handle. We don't have native_handle in win32.

libs\thread\test\sync\mutual_exclusion\recursive_timed_mutex\native_handle_pass.cpp(32) : fatal error C1189: #error :  "Test not applicable: BOOST_THREAD_DEFINES_RECURSIVE_TIMED_MUTEX_NATIVE_HANDLE not defined for this platform as not supported"
    call "C:\Users\appveyor\AppData\Local\Temp\1\b2_msvc_12.0_vcvarsall_x86.cmd" >nul
cl /Zm800 -nologo @"bin.v2\libs\thread\test\rec_timed_mutex_native_handle_p.test\msvc-12.0\debug\threading-multi\sync\mutual_exclusion\recursive_timed_mutex\native_handle_pass.obj.rsp" 
...failed compile-c-c++ bin.v2\libs\thread\test\rec_timed_mutex_native_handle_p.test\msvc-12.0\debug\threading-multi\sync\mutual_exclusion\recursive_timed_mutex\native_handle_pass.obj...

[karzhenkov]

When Boost.Thread is compiled with non-native <threadapi>, it marks the created library with the corresponding suffix _win32 or _pthread added to the name. This is implemented using the <tag> feature.

For Windows it's possible to use both options, and we'll get the names libboost_thread.dll (native variant, without <threadapi> suffix) and libboost_thread_pthread.dll.

On other platforms native variant (and perhaps the only one) is pthread, so the name must be libboost_thread.so. Name libboost_thread_pthread.so is incorrect; name libboost_thread_win32.so is probably not applicable.

Boost.Build can also add its own suffixes not considered here.

[viboes]

Thanks for the explanation about the tag.

Any comment about the regression for native_handle?

[viboes]

The native_handle test run as follows https://github.com/boostorg/thread/blob/develop/test/Jamfile.v2#L322

 [ thread-run2-noit-pthread ./sync/conditions/condition_variable/native_handle_pass.cpp : condition_variable__native_handle_p ]

where thread-run2-noit-pthread is defined as https://github.com/boostorg/thread/blob/develop/test/Jamfile.v2#L163

rule thread-run2-noit-pthread ( sources : name )
{
    sources = $(sources) winrt_init.cpp ;
    return
    [ run $(sources) ../build//boost_thread : : : <threadapi>win32:<build>no
      : $(name) ]
    [ run $(sources) ../src/tss_null.cpp ../build//boost_thread/<link>static
        : : : <threadapi>win32:<build>no
      : $(name)_lib ]
    #[ run $(sources) ../build//boost_thread : : :
    #  <define>BOOST_THREAD_DONT_PROVIDE_INTERRUPTIONS
    #  : $(name)_noit ]
    ;
}

It is like with your changed <threadapi>win32 was not defined.

I suspect that the default <threadapi>native has something to be with this.

Do you know how to fix this?

[karzhenkov]

I tried to change the usage-requirements, but now I can't build the library. The reason is not clear yet.

[karzhenkov]

Regression (I hope) is fixed. Perhaps there is a more elegant solution, but I just added the same detection logic into "test/Jamfile.v2".

[viboes]

Yes. This was the route cause and it fixes the issue for Boost.Thread tests.

We need however to add it on the usage-requirement, otherwise any other library that depends on Boost.Thread will have the same issue, isn't it?

[karzhenkov]

You're right. But I'm not an expert on Bjam too:) It turned out to be a bit more complicated. I will think.

[viboes]

Would you mind to check with the Boost.Build experts what can be done here? Steven Watanabe know all we could do with bjam.

[karzhenkov]

I would be grateful for expert advice:) Could Steven join the discussion?

So, we have:

1. Boost.Thread defines non-free feature <threadapi> and main target boost_thread. When boost_thread is built without specifying value of <threadapi> in the build request, the default value is used. It seems to be reasonable to make the default value dependent on <target-os>. This was intent of my revision 2fb41ed. I introduced additional value for <threadapi> (native) which became the default to represent "no value" (i. e. "no user request"). Using <conditional> in project requirements native is replaced with other value (either win32 or pthread) depending on <target-os>.
2. It is also desirable to have actual <threadapi> value (deduced by Boost.Thread or provided by user) in the property set used when any target dependent on boost_thread is built. It is even desirable to have it when Bjam is selecting the targets to build. For example, in libs/thread/test/Jamfile some targets are required not to build for <threadapi>win32. The attempt was made in my revision 6ee1c88. I added <conditional> to usage requirements but used wrong syntax and perhaps wrong approach. Usage requirements have specific traits:
   • they are added to property set of a target only after all its dependencies are built and therefore cannot affect target selection;
   • they don't replace properties of the target but are added to the property set (even for non-free features).

I'm not sure but the latter, probably, causes target to be build with each of feature values separately (for non-free features).

The questions:

• Is there any way to achieve the same effect as in 2fb41ed without introducing artificial <threadapi> value just to indicate "no value"?
• How to make feature value deduced by some target available in a project that uses this target as dependency?

Please correct me if I'm mistaken somewhere (especially in describing the problem).

[viboes]

I've send a mail to the Boost.Build ML (boost-build@lists.boost.org)

[viboes]

This looks quite good.

Andrey, what do you think about this PR?

[karzhenkov]

Hi,

<conditional>@threadapi.detect and import threadapi are needed only in projects that use actual <threadapi> value in the build process. You can take a look at this my exercise, subproject "my-thread-app-no-import". Note that for demo purposes <variant> (instead of <thread-os>) value is used to deduce default <threadapi>.

Definition of <threadapi> is moved now to "threadapi.jam". This should work and looks even better.

[viboes]

Ok, so we need to add some documentation for the projects that would use threadapi. Would you mind to add this documentation. I believe some Jamfile examples would help to the users.

If you don't know how to do it, you can post the text here. I believe that a new file cross_compiling.qbk included from overview.qbk would be good.

I don't see anything about the libraries that can be built depending on the parameters. It will be a good occasion to add it.

[Lastique]

Andrey, what do you think about this PR?

I'm not a Boost.Build expert, but the intention seems valid. I'm not sure I understood it though - will this change require modification of users' projects? There are a few dependent libraries in Boost and probably quite a few Boost.Build-based projects out there as well. It would be preferable if those continued to compile as they are.

[viboes]

The default has changed. Instead of been derived from the os.name it is derived from the . The deduced with the default is the same as the one deduced previously with os.name so I don't believe there will be any regression.

Now if the user set the target-os he will obtain the good library.

[viboes]

I've decided to merge it and see how it behaves on regression tests.

We could fix it later if there is some issue or rollback this change.

[viboes]

@karzhenkov Thanks you very much for working on this feature/fix.

[Lastique]

But then test/Jamfile.v2 wouldn't need to be modified, would it? I'm worried that the fact that it required changing means that the dependent projects will have to be modified as well. What's more alarming is that the change essentially is to perform the same kind of auto-detection that the library does, which arguably will be difficult to reproduce downstream. I think, the auto-detection should result in Boost.Thread usage requirement, which should be picked up by all downstream projects, including tests.

[viboes]

I believe that it needed changes because the intermediary change defaulted threadapi to native.

I believe that

https://github.com/boostorg/thread/blob/develop/test/Jamfile.v2#L21

and

https://github.com/boostorg/thread/blob/develop/test/Jamfile.v2#L110

shouldn't be needed now.

@karzhenkov could you confirm?

[viboes]

I've tried commenting these lines and everything is ok for the moment. The test takes some time.

[karzhenkov]

I think, usage requirements is unsuitable instrument here. They are intended for free features only (Boost.Build ML). And they are added to the property set after selecting target alternatives (The Build Process), This can be important.

A project that needs to know the underlaying threading API when building must contain <conditional>@threadapi.detect. Otherwise <threadapi> value will be present in the property set only if it is expicitly specified by user in the build request. For this reason, if you delete <conditional>@threadapi.detect from the test/Jamfile.v2, then the error that was discussed above will again occur.

Nevertheless, I believe that the impact of this PR on other projects will not be critical:

• In Boost there is no library that uses <theadapi> value, except for Boost.Thread.
• A project that uses specific functionality of underlying threading API is probably not portable. If so, it can contain hardcoded <threadapi> in requirements, or failure might be acceptable when building for alien platform.

[viboes]

Hi, well the test seems to say that we need to uncomment :(

https://ci.appveyor.com/project/viboes/thread/build/1.0.108-develop (there are other unrelated error, sorry) https://ci.appveyor.com/project/viboes/thread/build/1.0.111-develop

As you said , the native_handle error are there :(

This will mean that we have potentially break some users that use . Fortunately there is no Boost library using it.

I believe the number of concerned user should be small enough compared to the number of users that could benefit from this feature.

[viboes]

@karzhenkov We need to advertise this on the documentation. Have you time to work on this?

[viboes]

@karzhenkov The results confirm what you said.

https://ci.appveyor.com/project/viboes/thread/build/1.0.111-develop

[karzhenkov]

Well, I could spend some time to describe the potentially breaking changes. However, I would like to understand how dangerous they are.

The <threadapi> feature is documented in build/Jamfile.v2 only. Its declared purpose is to request a specific API. This is still correct.

The value of <threadapi> is processed in jam-files that belong to Boost.Build and translated into dependent projects as <define>, <include> and <library> values in usage requirements. Macro definitions provide all the necessary information to a C ++ programmer, and nothing changes here.

The changes concern users invoking b2 from command line and programmers authoring jam-files.

The former may provide <threadapi> value as before. User had to specify it in the case of cross-compilation. Now he is allowed to omit the explicit request. I think that it is not a breaking change, but the comment containing in build/Jamfile.v2 should be placed to overview.qbk (and somewhat expanded).

Perhaps some caveats are required for jam-programmers who may rely on undocumented behavior. I think that most likely there are no such projects.

Do we really have potentially breaking changes?

As of the work on the documentation in more general sense, I will consider this possibility, but it will not be too fast.

[viboes]

You are surely right, maybe there are no such project.

Don't worry for the documentation. I'll try to do my best. Thanks for all.

[kuhlenough]

Yes, I use on both Windows and Linux to cross compile for VxWorks. In the project-config.jam I consolidate all the cross-compile "feature" to avoid the long path lengths on windows when multiple features are specified on the b2 invocation. So ..

b2 cross-compile=vxworks

with

import option ;
import feature ;
import os ;

# composite feature to select all the cross compile options we need
# as one feature, so the directory path is a reasonable length, 
# primarily for windows host where path name length matter, and due to host
# pollution the correct options are not automatically selected

feature.feature cross-compile
    : vxworks
    : optional composite propagated 
    ;

feature.compose <cross-compile>vxworks 
    : <threadapi>pthread <binary-format>elf <abi>sysv <threading>multi <target-os>vxworks
    ;

Auto detection with cross-compilation while admirable is often broken, I'm happy if isn't confused with in the jam scripts.

[viboes]

Hi karzhenkov,

I have a report that the develop branch is broken. The user is doing the following in a linux platform

b2 --build-dir=dist --with-chrono --with-thread --with-system toolset=gcc link=static variant=release stage

and here it is what he gets

Performing configuration checks

    - 32-bit                   : no
    - 64-bit                   : yes
    - arm                      : no
    - mips1                    : no
    - power                    : no
    - sparc                    : no
    - x86                      : yes
    - symlinks supported       : yes
    - lockfree boost::atomic_flag : yes
error: Name clash for '<pstage/lib>libboost_system.a'
error: 
error: Tried to build the target twice, with property sets having 
error: these incompatible properties:
error: 
error:     -  none
error:     -  <threadapi>pthread
error: 
error: Please make sure to have consistent requirements for these 
error: properties everywhere in your project, especially for install
error: targets.

I'll try to reproduce this from a clean repository. How did you build Boost.Thread? Which command did you used?

From my side I just have run the tests from the test directory as

b2 toolset=gcc

and every thing worked correctly.

[viboes]

Adding threadapi=pthread to the command line solve the issue, but it shows that we have introduced a regression.

[karzhenkov]

Hi

I used standard command from Boost top-level directory:

./b2 toolset=gcc --with-thread

It works with gcc (mingw-w4) and with qcc (for QNX). The error occurs with the following command:

./b2 toolset=gcc --with-thread --with-system

This is a problem, and I have no solution yet.

This command requests the compilation of Boost.System in two versions. The first one is explicitly specified in the command and has no <threadapi> in property set. The second version is dependency of Boost.Thread and has <threadapi> value derived from <target-os>. Both versions use the same library name in the stage directory, and we get error: Name clash...

In fact, these versions are identical: Boost.System doesn’t know anything about the <threadapi> feature. However, Boost.Build threats them as distinct just because they have different property sets. If we add the right value for <threadapi> to the command line, the property sets for both requests will be the same, and Boost.Build will treat both versions as one. The error will disappear.

Maybe there is a way to tell Boost.Build not to create distinct version of Boost.System for different <threadapi> values? We need something like incidental attribute for <threadapi>, but incidental only for projects that do not depend on Boost.Thread. Theoretically, Boost.Build already has enough information to conclude that <threadapi> doesn’t affect Boost.System.

Another idea (also not ideal) is to include the <conditional>@threadapi.detect in a Jamroot. But I have some problems with jam-syntax, so I have not tested this.

Maybe the Boost.Build ML will help.

[viboes]

You can see my post to the ML

http://boost.2283326.n4.nabble.com/PR-to-cross-compiling-Boost-Thread-Help-needed-tt4698930.html

No response for the moment :(

I wonder how is it that the regression test are working. Are all the testers using threadapi=api?

If threadapiproperty cannot be deduced for Boost.Thread without requiring an explicit threadapi=api when other libraries as Boost.System are requested, what is the interest of deducing it for Boost.Thread?

[karzhenkov]

I think Bjam is not perfect. It can't scale well, as creates new builds for older projects when new ones define their own features.

Regression tests may succeed because they probably don't cause the stage target to be built. It is this target that leads to the name clash in the stage directory.

I propose to move <conditional>@threadapi.detect into Jamroot (commit 6958cb3 in Boost and commit 3e381c9 in Boost.Thread). Build errors will be fixed.

[karzhenkov]

@kuhlenough You probably get different names for Boost.Thread library when compile it on Windows and on Linux. And all the dynamic libraries that depend on Boost.Thread although they have the correct names, internally refer it with different names. Do you have such a problem?

[kuhlenough]

Yes, when compiling on Windows I add. b2 --abbreviate-paths --layout=system .. The layout is another thing that should be associated with target-os but is chosen by host-os ( perhaps for the cygwin ?)