Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[boost] Allow apple-clang version 10 for nowide and C++ 11 #3977

Merged
merged 4 commits into from
Jan 20, 2021
Merged

[boost] Allow apple-clang version 10 for nowide and C++ 11 #3977

merged 4 commits into from
Jan 20, 2021

Conversation

datalogics-kam
Copy link
Contributor

@datalogics-kam datalogics-kam commented Dec 23, 2020

Specify library name and version: boost/1.75.0

  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the
    conan-center hook activated.

Supporting information

This was my build configuration:

Configuration:
[settings]
arch=x86_64
arch_build=x86_64
build_type=Release
compiler=apple-clang
compiler.cppstd=11
compiler.libcxx=libc++
compiler.version=10.0
os=Macos
os.version=10.9
os_build=Macos
[options]
boost:i18n_backend=icu
[build_requires]
[env]
DEVELOPER_DIR=/Applications/Xcode-10.3.app/Contents/Developer
MACOSX_DEPLOYMENT_TARGET=10.9

Boost reported:

    - std::fstream is moveable and swappable : yes

apple-clang 10 is clang 4.2: Wikipedia table

...and clang 4.2 definitely supports all of C++ 11: Cppreference compiler support tables.

- Tested with apple-clang 10.0, which is equivalent to clang 4.2.

- Boost config reports that std::fstream is swappable and movable.
prince-chrismc
prince-chrismc previously approved these changes Dec 23, 2020
@conan-center-bot
Copy link
Collaborator

Some configurations of 'boost/1.69.0' failed in build 1 (15d65523b646f988c74000edd0dcb30585c85854):

gocarlos
gocarlos previously approved these changes Dec 24, 2020
mathbunnyru
mathbunnyru previously approved these changes Dec 24, 2020
@madebr
Copy link
Contributor

madebr commented Dec 25, 2020

@datalogics-kam
Can you perhaps take a look at #3992?

And tip while debugging this recipe: disable all other os'es and build only Release to get faster feedback of c3i.
Boost has really long build times.

@ghost ghost mentioned this pull request Dec 26, 2020
4 tasks
@SSE4
Copy link
Contributor

SSE4 commented Dec 28, 2020

the default mode of apple-clang 12 (and 10) is C++03:

clang++ --version
Apple clang version 12.0.0 (clang-1200.0.32.21)
Target: x86_64-apple-darwin19.5.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

clang++ -dM -E -x c++ /dev/null | grep __cplusplus
#define __cplusplus 199711L

you have to activate cppstd setting in order to use newer C++ standards (such as C++11), thus to get nowide building for apple clang

Copy link
Contributor

@SSE4 SSE4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should probably return None for apple-clang instead of 10/12

@dmn-star
Copy link
Contributor

@SSE4 The default C++ language standard in apple clang 12 is gnu++14 (~C++14 with GNU specific extensions).

man clang

then search "default C++"

@SSE4
Copy link
Contributor

SSE4 commented Dec 29, 2020

@SSE4 The default C++ language standard in apple clang 12 is gnu++14 (~C++14 with GNU specific extensions).

man clang

then search "default C++"

Does it seem like documentation is lying? I can't compile C++14 code without passing -std=c++14 explicitly.

@dmn-star
Copy link
Contributor

Trust no one. :-) Maybe Apple means XCode sets the option gnu++14 by default.

recipes/boost/all/conanfile.py Outdated Show resolved Hide resolved
recipes/boost/all/conanfile.py Outdated Show resolved Hide resolved
Co-authored-by: SSE4 <tomskside@gmail.com>
Co-authored-by: SSE4 <tomskside@gmail.com>
@dmn-star
Copy link
Contributor

dmn-star commented Jan 9, 2021

The boost packages on CCI are without nowide, fiber, numa, json, right?

@prince-chrismc
Copy link
Contributor

The boost packages on CCI are without nowide, fiber, numa, json, right?

The logs have not be uploaded yet (boost is huge and it takes time)

We'll need to wait before we can check them.

@SSE4 since you are following this PR, do you know if (the community) can pull packages from the internal "conan-upstream" remote?

@dmn-star
Copy link
Contributor

dmn-star commented Jan 9, 2021

  1. self.settings.compiler.cppstd is not set on CCI

  2. version_cxx11_standard_json = self._min_compiler_version_default_cxx11 is null ( apple-clang removed from _min_compiler_version_default_cxx11)

  3. self.options.without_fiber = True
    self.options.without_json = True
    self.options.without_nowide = True

but no one knows :-)

    >#nowide requires a c++11-able compiler + movable std::fstream: change default to not build on compiler with too old default c++ standard or too low compiler.cppstd
    #json requires a c++11-able compiler: change default to not build on compiler with too old default c++ standard or too low compiler.cppstd
    if self.settings.compiler.cppstd:
        if not tools.valid_min_cppstd(self, 11):
            self.options.without_fiber = True
            self.options.without_nowide = True
            self.options.without_json = True
    else:
        version_cxx11_standard_json = self._min_compiler_version_default_cxx11
        if version_cxx11_standard_json:
            if tools.Version(self.settings.compiler.version) < version_cxx11_standard_json:
                self.options.without_fiber = True
                self.options.without_json = True
                self.options.without_nowide = True
        else:
            self.options.without_fiber = True
            self.options.without_json = True
            self.options.without_nowide = True

@SSE4
Copy link
Contributor

SSE4 commented Jan 10, 2021

  1. self.settings.compiler.cppstd is not set on CCI

this is a WIP, we're going to add cppstd into CCI. probably not all of them, but only the most used and stable ones. right now, users who need to specify custom cppstd will have to build from source (the same as for every custom build settings or options).

@SSE4
Copy link
Contributor

SSE4 commented Jan 10, 2021

@SSE4 since you are following this PR, do you know if (the community) can pull packages from the internal "conan-upstream" remote?

I don't think so, AFAIK this is the only temporary repository that exists for a short period of time, and then removed to avoid wasting CI resources (disk space).

@SSE4
Copy link
Contributor

SSE4 commented Jan 10, 2021

The boost packages on CCI are without nowide, fiber, numa, json, right?

I think so, as well as without MPI, python, stack trace and several other libraries

@SSE4 SSE4 added the infrastructure Waiting on tools or services belonging to the infra label Jan 10, 2021
@SSE4
Copy link
Contributor

SSE4 commented Jan 10, 2021

@danimtb @jgsogo can you take a look why the build was marked as failed, while conan center bot reports success?

@dmn-star
Copy link
Contributor

dmn-star commented Jan 10, 2021

right now, users who need to specify custom cppstd will have to build from source (the same as for every custom build settings or options).

you know my point of view conan-io/conan#8304. Please rethink your "default" choice, do not use apple-clang in prehistoric c++003 mode. Set cppstd=14 for apple-clang (12) in boost recipe and the problem is fixed. And this is safe also because no one use apple-clang on command line.

as well as without MPI, python, stack trace and several other libraries

With the difference that graph_parallel, mpi, python are disabled by default but json, nowide, fiber are skipped because Conan think apple-clang not supported c++11 standard (#3977 (comment)).

default_options.update({"without_{}".format(_name): False for _name in CONFIGURE_OPTIONS})
default_options.update({"without_{}".format(_name): True for _name in ("graph_parallel", "mpi", "python")})

@datalogics-kam
Copy link
Contributor Author

Set cppstd=14 for apple-clang (12) in boost recipe and the problem is fixed. And this is safe also because no one use apple-clang on command line.

We're using apple-clang 10 at the moment, and from the command line. And while you can use an IDE to build our code, it's primarily built from the command line, from CMake projects.

Also, I don't think the language standard should be hardcoded in this recipe. Whatever gets passed to B2 should be from compiler.cppstd, or something from Conan itself that's also sent to Conan's built-in build helpers. That way, developers can be assured that the compiler is set up the same way for all the requirements in a build.

@dmn-star
Copy link
Contributor

I would wonder if you have not set the same c++ standard in your CMake projects that is set in Xcode 10. But agreed, the default c++ standard should rather be set everywhere by Conan itself.

@ghost ghost mentioned this pull request Jan 18, 2021
4 tasks
@dmn-star dmn-star mentioned this pull request Jan 19, 2021
4 tasks
@SSE4 SSE4 closed this Jan 20, 2021
@SSE4 SSE4 reopened this Jan 20, 2021
@conan-center-bot
Copy link
Collaborator

All green in build 7 (15e9b1be20a61892d5958ed494ab74a9d7b7b333)! 😊

@jgsogo
Copy link
Contributor

jgsogo commented Jan 20, 2021

About the compiler.cppstd: right now in conan-center-index we use whatever is the default for Conan, the reason is that we expect people installing Conan to be able to consume packages from ConanCenter without modifying anything. You install Conan, you run conan install boost/1.74.0 and you get a package. This is one value proposition we can't change.

Nevertheless, we talked to the hierarchy, and we are allowed to burn more resources if needed. So the plan is to generate packages for other compiler.cppstd values (and this is absolutely needed to fix some recipes, I refer you to this pull-request to read some dramatization about it). It is almost decided to generate those extra packages, but there are some caveats:

  • time: the more packages we generate, the more it takes to the CI to create all the packages, the more a contributor needs to wait for feedback in a pull-request.
  • random failures: from time to time we get a random failure from a build, compilation fails (memory, node failure, restarts,...). The more packages, the more choices of these failures.
  • graph and dependencies: to activate new configurations we need to be sure that all requirements have already been generated for those configurations, otherwise your PR will fail because of missing dependencies. We are already facing this problem trying to generate all the packages for some compilers (apple-clang 12.0 and modern gcc and clang in Linux). It is taking too much time and new references and revisions arrive and we need to start from the beginning again and again. We need to improve this process before adding new configurations.

Maybe the answer is not to generate ALL, but only some of the compiler.cppstd combinations. We are starting to gather information about the configurations requested by users in ConanCenter (conan-io/conan#8046), we expect to have some data before taking that decision (we need other teams to provide us with that information).

@jgsogo jgsogo removed the infrastructure Waiting on tools or services belonging to the infra label Jan 20, 2021
@conan-center-bot conan-center-bot merged commit 93b463c into conan-io:master Jan 20, 2021
@dmn-star
Copy link
Contributor

dmn-star commented Jan 20, 2021

@jgsogo Yes you will get the packages, but they are not actually consumable from Xcode. Because Xcode passes c++14 as standard by default.

@dmn-star
Copy link
Contributor

dmn-star commented Jan 20, 2021

And the next boost version can anyway no longer be compiled in C++03 mode at all.

As example:
./boost/math/tools/cxx03_warn.hpp:100:1: warning: CAUTION: Compiling Boost.Math in pre-C++11 conformance modes is now deprecated and will be removed from March 2021. [-W#pragma-messages]
BOOST_PRAGMA_MESSAGE("CAUTION: Compiling Boost.Math in pre-C++11 conformance modes is now deprecated and will be removed from March 2021.")

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants