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

[feature] generic tool to check for minumum compiler support #8002

Closed
1 task done
prince-chrismc opened this issue Nov 4, 2020 · 12 comments
Closed
1 task done

[feature] generic tool to check for minumum compiler support #8002

prince-chrismc opened this issue Nov 4, 2020 · 12 comments

Comments

@prince-chrismc
Copy link
Contributor

prince-chrismc commented Nov 4, 2020

There is this piece of boiler plate that get's copied a LOT in CCI, but it's also something large project that migrate to newer compilers need to handle as well...

Would this be something that could be added to conans.tools ?

    @property
    def _minimum_cpp_standard(self):
        return 14

    @property
    def _minimum_compilers_version(self):
        return {
            "Visual Studio": "15",
            "gcc": "5",
            "clang": "3.4",
            "apple-clang": "5.1",
        }

    def validate(self):
        if self.settings.compiler.get_safe("cppstd"):
            tools.check_min_cppstd(self, self._minimum_cpp_standard)
        min_version = self._minimum_compilers_version.get(str(self.settings.compiler))
        if not min_version:
            self.output.warn("{} recipe lacks information about the {} compiler support.".format(
                self.name, self.settings.compiler))
        else:
            if tools.Version(self.settings.compiler.version) < min_version:
                raise ConanInvalidConfiguration("{} requires C++{} support. The current compiler {} {} does not support it.".format(
                    self.name, self._minimum_cpp_standard, self.settings.compiler, self.settings.compiler.version))
@SSE4
Copy link
Contributor

SSE4 commented Nov 21, 2020

as this is quite a repeating patter in conan-center-index (right now, 10 occurrences for _minimum_compilers_version), I support to make a tool for it to help recipe authors.

@prince-chrismc
Copy link
Contributor Author

depending which substring you look for you get different lists! With different variations!

if tools.Version(self.settings.compiler.version) < min_version:
$ grep -r 'if tools.Version(self.settings.compiler.version) < min_version:'
recipes/cppcmd/all/conanfile.py:            if tools.Version(self.settings.compiler.version) < min_version:
recipes/cxxopts/all/conanfile.py:            if tools.Version(self.settings.compiler.version) < min_version:
recipes/folly/all/conanfile.py:            if tools.Version(self.settings.compiler.version) < min_version:
recipes/json-schema-validator/all/conanfile.py:            if tools.Version(self.settings.compiler.version) < min_version:
recipes/namedtype/all/conanfile.py:            if tools.Version(self.settings.compiler.version) < min_version:
recipes/skyr-url/all/conanfile.py:            if tools.Version(self.settings.compiler.version) < min_version:
recipes/tomlplusplus/all/conanfile.py:            if tools.Version(self.settings.compiler.version) < min_version:
recipes/twitch-native-ipc/all/conanfile.py:            if tools.Version(self.settings.compiler.version) < min_version:
recipes/twitchtv-libsoundtrackutil/all/conanfile.py:            if tools.Version(self.settings.compiler.version) < min_version:
_minimum_compilers_version
$ grep -r '_minimum_compilers_version'
recipes/capnproto/all/conanfile.py:    def _minimum_compilers_version(self):
recipes/capnproto/all/conanfile.py:        minimum_version = self._minimum_compilers_version.get(str(self.settings.compiler), False)
recipes/cpp-sort/all/conanfile.py:    def _minimum_compilers_version(self):
recipes/cpp-sort/all/conanfile.py:            min_version = self._minimum_compilers_version[str(compiler)]
recipes/cppcmd/all/conanfile.py:    def _minimum_compilers_version(self):
recipes/cppcmd/all/conanfile.py:        min_version = self._minimum_compilers_version.get(
recipes/cxxopts/all/conanfile.py:    def _minimum_compilers_version(self):
recipes/cxxopts/all/conanfile.py:        min_version = self._minimum_compilers_version.get(str(self.settings.compiler))
recipes/eastl/all/conanfile.py:    def _minimum_compilers_version(self):
recipes/eastl/all/conanfile.py:        mininum_compiler_version = self._minimum_compilers_version.get(str(self.settings.compiler))
recipes/folly/all/conanfile.py:    def _minimum_compilers_version(self):
recipes/folly/all/conanfile.py:        min_version = self._minimum_compilers_version.get(
recipes/namedtype/all/conanfile.py:    def _minimum_compilers_version(self):
recipes/namedtype/all/conanfile.py:        min_version = self._minimum_compilers_version.get(str(self.settings.compiler))
recipes/pfr/all/conanfile.py:    def _minimum_compilers_version(self):
recipes/pfr/all/conanfile.py:            min_version = self._minimum_compilers_version[str(compiler)]
recipes/skyr-url/all/conanfile.py:    def _minimum_compilers_version(self):
recipes/skyr-url/all/conanfile.py:        min_version = self._minimum_compilers_version.get(
recipes/tomlplusplus/all/conanfile.py:    def _minimum_compilers_version(self):
recipes/tomlplusplus/all/conanfile.py:        min_version = self._minimum_compilers_version.get(
minimum_required_compiler_version
$ grep -r 'minimum_required_compiler_version'
recipes/argparse/all/conanfile.py:            minimum_required_compiler_version = self._compiler_required_cpp17[str(self.settings.compiler)]
recipes/argparse/all/conanfile.py:            if tools.Version(self.settings.compiler.version) < minimum_required_compiler_version:
recipes/bitserializer/0.10/conanfile.py:            minimum_required_compiler_version = self._supported_compilers[str(self.settings.compiler)]
recipes/bitserializer/0.10/conanfile.py:            if tools.Version(self.settings.compiler.version) < minimum_required_compiler_version:
minimal_version
grep -r 'minimal_version = {'
recipes/bx/all/conanfile.py:        minimal_version = {
recipes/cmake/3.x.x/conanfile.py:        minimal_version = {
recipes/cpp-jwt/all/conanfile.py:        minimal_version = {
recipes/cpp-taskflow/all/conanfile.py:        minimal_version = {
recipes/dataframe/all/conanfile.py:        minimal_version = {
recipes/di/all/conanfile.py:        minimal_version = {
recipes/entt/3.x.x/conanfile.py:        minimal_version = {
recipes/frozen/all/conanfile.py:        minimal_version = {
recipes/fruit/all/conanfile.py:        minimal_version = {
recipes/function2/all/conanfile.py:        minimal_version = {
recipes/godot-cpp/all/conanfile.py:        minimal_version = {
recipes/libpqxx/all/conanfile.py:        minimal_version = {
recipes/restinio/all/conanfile.py:        minimal_version = {
recipes/sigslot/all/conanfile.py:        minimal_version = {
recipes/svgwrite/all/conanfile.py:        minimal_version = {
recipes/tabulate/all/conanfile.py:        minimal_version = {
recipes/taskflow/all/conanfile.py:        minimal_version = {
recipes/tl/all/conanfile.py:        minimal_version = {
recipes/uwebsockets/all/conanfile.py:        minimal_version = {

❗ I think this search is the best representative of the problem

$ grep -r 'self.output.warn(' | grep 'compiler' | wc -l
37
self.output.warn( + compiler
$ grep -r 'self.output.warn(' | grep 'compiler'
recipes/argparse/all/conanfile.py:            self.output.warn("This recipe has no support for the current compiler. Please consider adding it.")
recipes/bitserializer/0.10/conanfile.py:            self.output.warn("This recipe has no support for the current compiler. Please consider adding it.")
recipes/capnproto/all/conanfile.py:            self.output.warn("Cap'n Proto requires C++14. Your compiler is unknown. Assuming it supports C++14.")
recipes/celero/all/conanfile.py:            self.output.warn("celero requires C++14. Your compiler is unknown. Assuming it supports C++14.")
recipes/cpp-taskflow/all/conanfile.py:            self.output.warn("%s recipe lacks information about the %s compiler"
recipes/cppbenchmark/all/conanfile.py:            self.output.warn("cppbenchmark requires C++17. Your compiler is unknown. Assuming it supports C++17.")
recipes/cppcmd/all/conanfile.py:            self.output.warn("{} recipe lacks information about the {} compiler support.".format(
recipes/cppcommon/all/conanfile.py:            self.output.warn("cppcommon requires C++17. Your compiler is unknown. Assuming it supports C++17.")
recipes/cxxopts/all/conanfile.py:            self.output.warn("{} recipe lacks information about the {} compiler support.".format(
recipes/elfutils/all/conanfile.py:            self.output.warn("Compiler %s is not gcc." % self.settings.compiler)
recipes/folly/all/conanfile.py:            self.output.warn("{} recipe lacks information about the {} compiler support.".format(
recipes/h5pp/all/conanfile.py:            self.output.warn("h5pp requires C++17. Your compiler is unknown. Assuming it supports C++17.")
recipes/hana/all/conanfile.py:            self.output.warn("This recipe might not support the compiler. Consider adding it.")
recipes/json-schema-validator/all/conanfile.py:            self.output.warn("{} recipe lacks information about the {} compiler support.".format(
recipes/magic_enum/all/conanfile.py:            self.output.warn("magic_enum requires C++17. Your compiler is unknown. Assuming it supports C++17.")
recipes/morton-nd/all/conanfile.py:            self.output.warn("morton-nd requires C++14. Your compiler is unknown. Assuming it supports C++14.")
recipes/ms-gsl/all/conanfile.py:            self.output.warn("ms-gsl requires C++14. Your compiler is unknown. Assuming it supports C++14.")
recipes/namedtype/all/conanfile.py:            self.output.warn("{} recipe lacks information about the {} compiler support.".format(
recipes/nanodbc/all/conanfile.py:            self.output.warn("nanodbc requires c++14, but is unknown to this recipe. Assuming your compiler supports c++14.")
recipes/openblas/all/conanfile.py:            self.output.warn("Building with lapack support requires a Fortran compiler.")
recipes/packio/all/conanfile.py:            self.output.warn("packio requires C++17. Your compiler is unknown. Assuming it supports C++17.")
recipes/pprint/all/conanfile.py:            self.output.warn("pprint needs a c++17 capable compiler")
recipes/quill/all/conanfile.py:            self.output.warn("Quill requires C++14. Your compiler is unknown. Assuming it supports C++14.")
recipes/simdjson/all/conanfile.py:            self.output.warn("{} requires C++17. Your compiler is unknown. Assuming it supports C++17.".format(self.name))
recipes/skyr-url/all/conanfile.py:            self.output.warn("{} recipe lacks information about the {} compiler support.".format(
recipes/sol2/3.x.x/conanfile.py:            self.output.warn("sol2 requires C++17. Your compiler is unknown. Assuming it supports C++17.")
recipes/spy/all/conanfile.py:            self.output.warn("Spy requires C++17. Your compiler is unknown. Assuming it supports C++17.")
recipes/structopt/all/conanfile.py:            self.output.warn("{} recipe lacks information about the {} compiler standard version support".format(self.name, compiler))
recipes/svgwrite/all/conanfile.py:            self.output.warn("{} recipe lacks information about the {} compiler"
recipes/swig/all/conanfile.py:            self.output.warn("Visual Studio compiler cannot create ccache-swig. Disabling ccache-swig.")
recipes/tabulate/all/conanfile.py:            self.output.warn("%s recipe lacks information about the %s compiler"
recipes/taocpp-json/all/conanfile.py:            self.output.warn("taocpp-json requires C++17. Your compiler is unknown. Assuming it supports C++17.")
recipes/taocpp-taopq/all/conanfile.py:            self.output.warn("taocpp-taopq requires C++17. Your compiler is unknown. Assuming it supports C++17.")
recipes/tomlplusplus/all/conanfile.py:            self.output.warn("{} recipe lacks information about the {} compiler support.".format(
recipes/twitch-native-ipc/all/conanfile.py:            self.output.warn("unknown compiler, assuming C++17 support")
recipes/twitchtv-libsoundtrackutil/all/conanfile.py:            self.output.warn("unknown compiler, assuming C++17 support")
recipes/zxing-cpp/all/conanfile.py:            self.output.warn("This recipe might not support the compiler. Consider adding it.")

prince-chrismc added a commit to prince-chrismc/user-management that referenced this issue Nov 29, 2020
now it generated in one place

little fancy on the cmake with private features, requires C++ 17 compiler, conanfile needs conan-io/conan#8002
prince-chrismc added a commit to prince-chrismc/user-management that referenced this issue Dec 5, 2020
author Chris McArthur <[email protected]> 1606357445 -0500
committer Chris McArthur <[email protected]> 1607133653 -0500

minimal poc linux only server logging

refactor how server header is populated

now it generated in one place

little fancy on the cmake with private features, requires C++ 17 compiler, conanfile needs conan-io/conan#8002

⚠️ do not update lockfile!

add version and settings when making locks

avoiding funky namespace declaraiton error
prince-chrismc added a commit to prince-chrismc/user-management that referenced this issue Dec 11, 2020
minimal poc linux only server logging

* parent f6712ed
author Chris McArthur <[email protected]> 1606357445 -0500
committer Chris McArthur <[email protected]> 1607133653 -0500

refactor how server header is populated

now it generated in one place

little fancy on the cmake with private features, requires C++ 17 compiler, conanfile needs conan-io/conan#8002

⚠️ do not update lockfile!

add version and settings when making locks

avoiding funky namespace declaraiton error

* adding cmake option to configure logging method

* fail bad logging options

* add logging option to conan file

* make sure to pass option list correctly

* initial pass for user databse logging

* fixing database logger

* [refactor] logger to be more generic

* add missing keyword for cmake

* adding logging to the main handlers

* build debug console logging for quick testing!

* fix bad value
@prince-chrismc
Copy link
Contributor Author

Recently we noticed the version compare was failing when settings.compiler.version = 6 and was minimum_version = 6.4

Now everywhere where the aforementioned code exists could benefit from the following blob

https://github.com/conan-io/conan-center-index/pull/3472/files#diff-
f262a4dd3c2a23c9985dbbcf5ed0ae18bc3b8bca3fa0f80ffa8dcad9c075b421R32-R38

conan-center-bot pushed a commit to conan-io/conan-center-index that referenced this issue Dec 29, 2020
* Add initial [Boost-ext].UT recipe

* Lowercase package name

* Remove non-ascii character from description

* Fix URL

* Use source folder variable and include the Boost license

* Remove CMake generator

* Remove export_sources directive

* Copy the header file to the correct location

* Use explicit test_package name for the test executable target

* Disable C++ compiler extensions

* Remove includedirs property

* Format Python code according to PEP8

* Require minimum compiler versions and at least the C++20 standard

* Rename ut to boost-ext-ut

* Check if the cppstd is set before checking the minimum

* Provide warnings to consumers when compiler checks fail

Code taken from: conan-io/conan#8002

* Rename ut directory to boost-ext-ut

* Chang the minimum C++ standard to 17

Previously, the minimum C++ standard was 20, but only 17 is required.

* Import ConanInvalidConfiguration

* Configure CMake for using find_package

The behavior now models [Boost-ext].UT's current find_package behavior.
A CMake config file is provided with the name ut.
The library can be found with find_package(ut) as expected.
The CMake target, boost::ut, is also provided as expected.
I'm not sure how to remove the global target boost::boost.
I'd hate for this to conflict with the Boost library.

* Shorten line lengths to less that 80 characters for PEP8

* Remove unused import

The glob import was carried over from another header-only package.
Since it is not used here, it has been removed.

* Use the proper identifier for the Boost license

The license was previously "Boost".
This commit switches the license to proper identifier for this license.
The short identifier for the Boost license is BSL-1.0.
This comes from the SPDX specification:
https://spdx.org/licenses/BSL-1.0.html
conan-center-bot pushed a commit to conan-io/conan-center-index that referenced this issue Feb 8, 2021
* Add recipe for CERN ROOT data analysis framework.

* Minimal changes to fix CI failure.

Fixes two classes of CI failure:

(1) ROOT6 requires C++ standard >= 11.
This is now checked with tools.check_min_cppstd.

(2) Seg fault when building with clang dues LLVM library loading issues.
See: https://root-forum.cern.ch/t/root-with-geant4-llvm-problems/21960

* Implement improvements for PR #3732

Implement improvements suggested by @prince-chrismc including:

(1) Remove default options for dependent packages.

(2) Remove all f-strings for python 2 compatibility.

(3) Cache CMake object.

(4) Use dependencies from CCI where available.

(5) Reduce size of test_package.

(6) Apply version check boiler plate from conan-io/conan#8002.

* Set CMAKE_CXX_STANDARD in root/all/test_package.

* Implement PR #3732 review improvements

(1) Remove unused variable.

(2) Prefer class variables over instance variables.

(3) Move source patching from source method to build method.

(4) String formatting style changes.

(5) Use tools to remove files over in-recipe implementation.

* Fix typo in 66718b6.

This should fix the failing CI clang-5 build.

* Specify `required_conan_version`

Co-authored-by: Chris Mc <[email protected]>

* Update openssl version number for ROOT

Co-authored-by: Anonymous Maarten <[email protected]>

* Fix backslashes in paths that might cause problems on Windows

Co-authored-by: Anonymous Maarten <[email protected]>

* On Windows, backslashes in paths might cause problems (escaping of variables)

Co-authored-by: Anonymous Maarten <[email protected]>

* Adding FIXME comment for dependencies where there is currently no Conan Center Index dependency available.

* Include conanbuildinfo.cmake in ROOT CMake as this is needed for MSVC.

Following recommendation in comments for #3732 by @madebr:
"This cmake script sets the current C runtime on MSVC (MT vs MD vd MTd vs MDd).
It also sets libstdc++11 vs libstdc++."

* Fix typo in 8b41c8a

* Fix typo in cmake.

* Fix path to source folder, solves issue when it is not current working directory.

* Fix failing build causing by changes for MSVC (introduced in 9181355)

* Rename package from root to cern-root.

* Apply suggestions from code review

Use self._source_subfolder everywhere
use os.path.join(...) instead of os.sep.join((...))

Co-authored-by: Anonymous Maarten <[email protected]>

* Fix typo in previous commit.

* Fix build failure due to moving build directory in 13df472

* Fix LICENSE file copying to output package.

* Apply suggestions from code review

Update dependency version numbers.
Change name of SQLite3 target.

Co-authored-by: SpaceIm <[email protected]>

* Run black on cern-root conanfile.py.

* Update ROOT version to latest patch (v6-22-06).

* Rename TODO to FIXME as FIXME is CCI convention.

* Add FIXME to warn about multiple CMake files as discussed in #3732 (comment)

* Fix issue with the wrong path when conan is called with --source-folder argument.

* Remove dead code.

* Reinstate conan_basic_setup.

The previous issues with conan_basic_setup (discussed in #3732 (comment))
were caused by conan_output_dirs_setup(). We now disable this step by
calling "conan_basic_setup(NO_OUTPUT_DIRS)".

Remove explicit setting of CMAKE_CXX_STANDARD as conan_basic_setup should set CMAKE_CXX_STANDARD for us.

* Revert change to CXX standard in 6548864f

Previous commit was incorrect and caused failures in clang <= 4.
conan_basic_setup will only set the standard under certain conditions
and this is experimental.
See: https://docs.conan.io/en/1.7/howtos/manage_cpp_standard.html

Revert to the original method of setting this.

* Implement style suggestions from @SpaceIm's  code review.

* Require libstdc++11. ROOT will not build with out the new ABI.

* Fix issue with the wrong path when conan is called with --source-folder argument.

* Try an alternative solution to the libcxx problem.

* Remove version number from file.

Co-authored-by: Chris Mc <[email protected]>

* Adding code review suggestions by @madebr

This should solve build issues on gcc and clang when libcxx != libstdc++11.

See:
<#3732 (comment)>

* Raise error on libc++ as it doesn't build due to portability issues in ROOT.

* Fix test_package CMake "install FILES given no DESTINATION" error.

Co-authored-by: Chris Mc <[email protected]>
Co-authored-by: Anonymous Maarten <[email protected]>
Co-authored-by: SpaceIm <[email protected]>
@jgsogo
Copy link
Contributor

jgsogo commented Feb 19, 2021

IMHO, we already have this functionality implemented in the check_min_cppstd tool. This necessity arises because we are not using profiles with different cppstd values but the default ones, don't you think so?

@prince-chrismc
Copy link
Contributor Author

prince-chrismc commented Feb 19, 2021

Yes, I think that would cover the majority of use cases.

There's a very slim number which require minimum versions because the CXX standard in some compilers is only partial, just because you set C++17 does not mean it's all there.

EDIT: This happens conan-io/conan-center-index#4631

@Croydon
Copy link
Contributor

Croydon commented Feb 19, 2021

Having a check_min_compiler_version (or something like that) could still prevent this boilerplate in such cases. CCI had quite some recipe bugs because of bugs in these checks

And not all consumers have the cppstd in their profiles either

@sourcedelica
Copy link

I agree that not all consumers have cppstd in their profiles. We don't so far. It seems to me that the C++ standard belongs to the recipe, not the profile. I use one profile to build packages with different C++ standard requirements. Some require C++11, others C++14, others C++17.

@sourcedelica
Copy link

I am currently working on a recipe where some versions of the package require C++11 and later versions require C++17. It would be good if this tool supported that - for example if _minimum_compilers_version() took a cppstd parameter instead of using a hardcoded _minimum_cpp_standard().

@sourcedelica
Copy link

For example:

def _minimum_compilers_version(self, cppstd):
    standards = {
        "11": {
            "Visual Studio": "15",
            "gcc": "4.8",
            "clang": "4",
            "apple-clang": "9",
        },
        "17": {
            "Visual Studio": "16",
            "gcc": "7",
            "clang": "5",
            "apple-clang": "10",
        },
    }
    return standards.get(cppstd) or {}

def validate(self):
    cppstd = "11" if Version(self.version) <= "0.17.6" else "17"
    if self.settings.compiler.get_safe("cppstd"):
        tools.check_min_cppstd(self, cppstd)
    min_version = self._minimum_compilers_version(cppstd).get(str(self.settings.compiler))
    if not min_version:
        self.output.warn("{} recipe lacks information about the {} compiler support.".format(
            self.name, self.settings.compiler))
    else:
        if tools.Version(self.settings.compiler.version) < min_version:
            raise ConanInvalidConfiguration("{} requires C++{} support. The current compiler {} {} does not support it.".format(
                self.name, cppstd, self.settings.compiler, self.settings.compiler.version))

@Minimonium
Copy link
Contributor

I have a PR open for quite a long time with a comprehensive implementation. While it provides a lot of different things - you can salvage a smaller scope tool.

#6713

@Minimonium
Copy link
Contributor

Also related to conan-io/conan-center-index#54

@AbrilRBS
Copy link
Member

Closing as not planned - As CCI reviewers, we're ok with recipes repeating this pattern where necessary. Note that in CCI, it's mostly used for cppstd checks in Conan 1. Conan 2 models this property way better, so that the check becomes irrelevant in most cases.

The cases that would still need it would not benefit from having this logic abstracted out, because as commented here, there are lots of variations of the expected behaviour

Thanks everyone for your input!

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

No branches or pull requests

8 participants