Skip to content

simple-yaml: add compatibility for older compilers #9063

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

Merged
merged 4 commits into from
Feb 15, 2022

Conversation

Zeeno-atl
Copy link
Contributor

@Zeeno-atl Zeeno-atl commented Jan 24, 2022

Specify library name and version: simple-yaml/0.2.1

The previous PR #8705 didn't generate any packages due to the fact that no compiler in the PR pipeline supported <source_location> header. Adding compatibility library source_location solves this issue and thus it should generate some packages this time (hopefully). And since there were no packages online, I also bumped the version removing the older one.


  • 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.

@Zeeno-atl Zeeno-atl changed the title simple-yaml: add compatibility to older compilers simple-yaml: add compatibility for older compilers Jan 24, 2022
@conan-center-bot

This comment has been minimized.

@Zeeno-atl
Copy link
Contributor Author

Zeeno-atl commented Jan 24, 2022

I need a cap here. It took me a while to figure out what is going on, but now I guess I understand. The Docker image a clang11 is based on is FROM ubuntu:bionic. This ships with the libstdc++-7.5 . The conan/cmake compiles the code with GCC's libstdc++-7.5 header files. This means that even though the <filesystem> header is supported by Clang's libc++ since version 7 and GCC's libstdc++ since version 8, the compilation fails.

In the Dockerfile there is a

    # Further LLVM packages not installed by the llvm.sh script
    && apt-get install -y --no-install-recommends --no-install-suggests \
       libc++-11-dev \
       libc++abi-11-dev \

attempt to install libc++-11, but it is in fact not used by the compiler. I am not sure where the error is.
Should conan set linking against libc++ (-stdlib=libc++) instead of libstdc++ when compiling using Clang/LLVM?
Should Dockerfile include a bit more recent libstdc++ when compiling with a recent Clang/LLVM version?
If both of the answers are NO, what is the point of using a recent compiler with archaic standard libraries?

EDIT: I guess it is not only linking, but setting include folders as well to the proper version of header files. I am a mere user of build systems, not sure how to set things up.

@SSE4
Copy link
Contributor

SSE4 commented Jan 25, 2022

seems like more or less the same problem as with #8927
in general, it seems like libstdc++ and clang versions are orthogonal and independent, and probably it makes sense to install the latest (or at least more recent) libstdc++ version on clang machines. but that may break binary compatibility (needs to be carefully checked, if binaries will require new version of libstdc++.so, and if they will be able to run on the vanilla distro).

@SSE4 SSE4 added the infrastructure Waiting on tools or services belonging to the infra label Jan 25, 2022
@uilianries
Copy link
Member

It's a peculiar situation, and we have few options here, let me list them:

  • Update Docker image conanio/clang11 with a newer libstdc++ version
    • It should fix your problem, but at a high risk of breaking other packages. Conan Center has tons of packages that we can't validate possible side effects, like runtime errors.
  • Switch the Docker images from conanio/clang11 to conanio/clang11-ubuntu16.04
    • The newer Docker images are running more aligned compilers with their respective libstdc++ and libc++ versions. However, the current conanio/clang11 uses Ubuntu 18:04 which contains a newer version of glibc, so can not do it, otherwise it would break all packages in Conan Center.
  • Raise InvalidConfigurationError and ignore Clang11 for now
    • At some point, Clang 14 will be released and Clang11 will no longer produce packages in Conan Center

We understand the situation, but we can not risk the current stability due this need. Unfortunately, we will need to skip Clang 11 for now and wait for Clang 13 become stable, then, stop to use Clang 11.

@SSE4
Copy link
Contributor

SSE4 commented Jan 25, 2022

yes, for now, we can't change clang11 docker image, so let's add the following code snippet to affected recipes:

def validate(self):
    ....
    if self.settings.compiler == "clang" and self.settings.compiler.libcxx in ["libstdc++", "libstdc++11"] and self.settings.compiler.version == "11":
        raise ConanInvalidConfiguration("clang 11 with libstdc++ is not supported due to old libstdc++ missing C++17 support")

this way, it will skip clang11 + libstdc++ configuration, but will still build clang11 + libc++.

@SSE4 SSE4 mentioned this pull request Jan 25, 2022
4 tasks
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot
Copy link
Collaborator

All green in build 4 (f5e0cda69f8d1e1699246210198afb54fef17480):

  • simple-yaml/0.2.2@:
    All packages built successfully! (All logs)

@SSE4 SSE4 mentioned this pull request Feb 1, 2022
4 tasks
@ghost
Copy link

ghost commented Feb 15, 2022

I detected other pull requests that are modifying simple-yaml/all recipe:

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

@conan-center-bot conan-center-bot merged commit 2900159 into conan-io:master Feb 15, 2022
@jgsogo
Copy link
Contributor

jgsogo commented Feb 16, 2022

I've seen there had been a problem in the past uploading 0.2.0 and that version was not available in the center. Version 0.2.2 has been successfully uploaded and it is available.

I think we are fine, no need to recover the old version.

Thanks for your contribution here!

SSE4 pushed a commit to madebr/conan-center-index that referenced this pull request Feb 21, 2022
* fix(simple-yaml): add source_location package to lower the compiler demands

* fix: disable clang11 + libstdc++ combination due to improper compilation/linking

* bump(simple-yaml): version 0.2.2

* fix(simple-yaml): apple-clang template specialization by concept not supported in v12
This was referenced Mar 9, 2022
feltech added a commit to OpenAssetIO/conan-center-index that referenced this pull request Jan 10, 2023
The ConanCenter CI runners for Clang 11 apparently don't have a
libstdc++ that contains `std::filesystem`, according to e.g.
 * conan-io#10043 (comment))
 * conan-io#9063 (comment)

Other packages seem to have solved this by bumping the minimum allowed
to Clang 12.

Signed-off-by: David Feltell <[email protected]>
feltech added a commit to OpenAssetIO/conan-center-index that referenced this pull request Mar 21, 2023
The ConanCenter CI runners for Clang 11 apparently don't have a
libstdc++ that contains `std::filesystem`, according to e.g.
 * conan-io#10043 (comment))
 * conan-io#9063 (comment)

Other packages seem to have solved this by bumping the minimum allowed
to Clang 12.

Signed-off-by: David Feltell <[email protected]>
conan-center-bot pushed a commit that referenced this pull request Apr 12, 2023
* Add OpenAssetIO v1.0.0-alpha.7

OpenAssetIO is a C++ library with C and Python bindings. It has a plugin
system allowing so-called "manager" plugins to be loaded into a "host"
application.  Hosts and plugins can be written in C++, C or Python, with
OpenAssetIO marshalling between languages as necessary.

The goal is to provide a common interface for host applications to
communicate with a central asset management system - a common pattern in
the VFX industry.

The project is currently in the alpha stage of development.

The Conan recipe has a few uncommon features:

The `fPIC` option is not supported.  Since one of the build artifacts is
a Python extension module, which must be a shared library and must link
to the core C++ library, we must always enable PIC for the core library.

The `cpython` package is used as a dependency. It's surprising how few
(none, apparently) recipes make use of the `cpython` Conan package. The
`cpython` package is not currently Conan v2 friendly, so we must use,
for example, the deprecated `deps_user_info`.

The C bindings, if built as a static library, must be told to link to
the C++ standard library. The way Conan's CMake generators work
(`IMPORTED INTERFACE` rather than `IMPORTED STATIC`) means that this
isn't done automatically. To work around this we take inspiration from
#729.

The `test_package` uses an external test repository that exercises basic
linkage and imports. This is also used upstream as one of the CI checks.

Signed-off-by: David Feltell <[email protected]>

* Ensure new-enough CMake is used for building.

`cmake` requires `openssl/1.1.1s` whereas `cpython` requires
`openssl/1.1.1l`, causing "Conflict in cmake/3.21.7" build error. So
override `openssl` dependency in the root conanfiles to the newer
version.

Signed-off-by: David Feltell <[email protected]>

* Make tomlplusplus a `requires` rather than `tool_requires`

For some reason `tomlplusplus` is now no longer found when added as a
`tool_requires`, but it should probably be a `requires` anyway, based
on the Conan docs (even though it's not part of the public API/ABI).

Signed-off-by: David Feltell <[email protected]>

* Work around cpython recipe clang pthread flag bug

The current cpython recipe has a bug, in that it artificially adds
`-lpthread` to `LD_FLAGS` in autotools, so that the Python `configure`
script thinks it doesn't need to add `-lpthread` itself, and so doesn't
store `-lpthread` in the list of required system libraries in the
generated `sysconfig` Python module.

CMake's bundled `FindPython` module uses `sysconfig` to get the system
libraries required for linking an embedded Python interpreter.  But
because of the above, `-lpthread` is missing from this list.

Signed-off-by: David Feltell <[email protected]>

* Bump minimum clang version

The ConanCenter CI runners for Clang 11 apparently don't have a
libstdc++ that contains `std::filesystem`, according to e.g.
 * #10043 (comment))
 * #9063 (comment)

Other packages seem to have solved this by bumping the minimum allowed
to Clang 12.

Signed-off-by: David Feltell <[email protected]>

* Disable MacOS support

OpenAssetIO relies on CMake's `FindPython.cmake` module, but that has
issues when used with the `cpython` Conan package, in that `FindPython`
assumes the include directories of `cpython`'s dependencies are
discoverable in the system include paths, rather than the conan cache.

This causes `fatal error: 'crypt.h' file not found` errors on MacOS,
for more info see OpenAssetIO/OpenAssetIO#722

Signed-off-by: David Feltell <[email protected]>

* Work around cpython `rootpath` being unavailable on Windows

For some reason `dependencies["cpython"].cpp_info.rootpath` is currently
unavailable on Windows (works on Linux), so resort to using the
deprecated `deps_cpp_info` instead, on the assumption that this is a
Conan bug/omission on the road to 2.0.

* Fix Python executable path on Windows

Either the backslashes need to be escaped, or we can just use a posix
path. Otherwise CMake complains about unknown escape sequences.

* Fix Python library path for Windows

The library in `cpp_info.libs` is the library name, not the library
filename, so we must add the file extension.

We must also then convert the path to posix to get rid of the
backslashes.

* Work around cpython package static build missing import library

In the static build of cpython the `python39.lib` file is,
understandably, a static library rather than a Windows dll import
library.

When building a Python extension module we require the import library,
since the extension module itself should not statically link the whole
of Python(!).

So switch to the `shared` variant of cpython for Windows.

* Rename option from `without_python` to `with_python`

Signed-off-by: David Feltell <[email protected]>

* Move enforcement of `cpython:shared=True` to `configure` method

Signed-off-by: David Feltell <[email protected]>

* Bump OpenAssetIO version to alpha.8

Signed-off-by: David Feltell <[email protected]>

* Bump required CMake version to latest

Dependencies may want a newer version than 3.21, but if we're the root
recipe then we'll override it.

Signed-off-by: David Feltell <[email protected]>

* Bump require pybind11 version

New patch version now available.

Signed-off-by: David Feltell <[email protected]>

* Simplify CXX11_ABI CMake option computation

Slight change of behaviour, since the option will now always be set, but
doesn't do any harm.

Signed-off-by: David Feltell <[email protected]>

* Remove erroneous `cpp_info.libs` entry

The list of libraries is broken down by component, so just blank the
top-level list.

Signed-off-by: David Feltell <[email protected]>

* Use `package_folder` when computing paths

Use the more modern `package_folder` property, rather than `rootpath`.

Signed-off-by: David Feltell <[email protected]>

* Add TODO re. `cpython` recipe V2 incompatibility

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

* Add `validate` error if MSVC and not `cpython:shared=True`

Although we force `cpython:shared=True` in `configure()`, that may be
overridden by another package in the dependency tree.

Signed-off-by: David Feltell <[email protected]>

* Update `validate()` to match updated package template

i.e. `self.info.settings` -> `self.settings`, to match
`docs/package_templates/cmake_package/conanfile.py`

Signed-off-by: David Feltell <[email protected]>

* Simplify `test_package`

Bundle the test project, rather than fetching from an external source,
and simplify its contents (e.g. no C API tests, assume correct Python
version, ...).

Signed-off-by: David Feltell <[email protected]>

* Bump OpenAssetIO version to 1.0.0-alpha.9

Another release happened in the meantime. Note that the (incomplete)
C API bindings are no longer built by default, so that Conan component
has been removed.

Signed-off-by: David Feltell <[email protected]>

* Remove unused `_stdcpp_library` now that C API is no longer built

Signed-off-by: David Feltell <[email protected]>

* Remove unused import

Signed-off-by: David Feltell <[email protected]>

* Simplify C++ test

Signed-off-by: David Feltell <[email protected]>

* Override ncurses version to one that has pre-built package available

Signed-off-by: David Feltell <[email protected]>

* Remove unnecessary `source_folder` argument to `get`

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

* Remove unnecessary `f`-string

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

* Remove unnecessary set of `generate`'s `scope` arg

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

* Remove `openssl` override now its no longer needed

Signed-off-by: David Feltell <[email protected]>

* Add missing `package_type` metadata

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

* Remove `python_version` option

If consumers want an OpenAssetIO built for a different Python version,
then they can override the `cpython` dependency to a different version.

Signed-off-by: David Feltell <[email protected]>

---------

Signed-off-by: David Feltell <[email protected]>
Co-authored-by: Chris Mc <[email protected]>
MartinDelille pushed a commit to MartinDelille/conan-center-index that referenced this pull request Apr 12, 2023
* Add OpenAssetIO v1.0.0-alpha.7

OpenAssetIO is a C++ library with C and Python bindings. It has a plugin
system allowing so-called "manager" plugins to be loaded into a "host"
application.  Hosts and plugins can be written in C++, C or Python, with
OpenAssetIO marshalling between languages as necessary.

The goal is to provide a common interface for host applications to
communicate with a central asset management system - a common pattern in
the VFX industry.

The project is currently in the alpha stage of development.

The Conan recipe has a few uncommon features:

The `fPIC` option is not supported.  Since one of the build artifacts is
a Python extension module, which must be a shared library and must link
to the core C++ library, we must always enable PIC for the core library.

The `cpython` package is used as a dependency. It's surprising how few
(none, apparently) recipes make use of the `cpython` Conan package. The
`cpython` package is not currently Conan v2 friendly, so we must use,
for example, the deprecated `deps_user_info`.

The C bindings, if built as a static library, must be told to link to
the C++ standard library. The way Conan's CMake generators work
(`IMPORTED INTERFACE` rather than `IMPORTED STATIC`) means that this
isn't done automatically. To work around this we take inspiration from
conan-io#729.

The `test_package` uses an external test repository that exercises basic
linkage and imports. This is also used upstream as one of the CI checks.

Signed-off-by: David Feltell <[email protected]>

* Ensure new-enough CMake is used for building.

`cmake` requires `openssl/1.1.1s` whereas `cpython` requires
`openssl/1.1.1l`, causing "Conflict in cmake/3.21.7" build error. So
override `openssl` dependency in the root conanfiles to the newer
version.

Signed-off-by: David Feltell <[email protected]>

* Make tomlplusplus a `requires` rather than `tool_requires`

For some reason `tomlplusplus` is now no longer found when added as a
`tool_requires`, but it should probably be a `requires` anyway, based
on the Conan docs (even though it's not part of the public API/ABI).

Signed-off-by: David Feltell <[email protected]>

* Work around cpython recipe clang pthread flag bug

The current cpython recipe has a bug, in that it artificially adds
`-lpthread` to `LD_FLAGS` in autotools, so that the Python `configure`
script thinks it doesn't need to add `-lpthread` itself, and so doesn't
store `-lpthread` in the list of required system libraries in the
generated `sysconfig` Python module.

CMake's bundled `FindPython` module uses `sysconfig` to get the system
libraries required for linking an embedded Python interpreter.  But
because of the above, `-lpthread` is missing from this list.

Signed-off-by: David Feltell <[email protected]>

* Bump minimum clang version

The ConanCenter CI runners for Clang 11 apparently don't have a
libstdc++ that contains `std::filesystem`, according to e.g.
 * conan-io#10043 (comment))
 * conan-io#9063 (comment)

Other packages seem to have solved this by bumping the minimum allowed
to Clang 12.

Signed-off-by: David Feltell <[email protected]>

* Disable MacOS support

OpenAssetIO relies on CMake's `FindPython.cmake` module, but that has
issues when used with the `cpython` Conan package, in that `FindPython`
assumes the include directories of `cpython`'s dependencies are
discoverable in the system include paths, rather than the conan cache.

This causes `fatal error: 'crypt.h' file not found` errors on MacOS,
for more info see OpenAssetIO/OpenAssetIO#722

Signed-off-by: David Feltell <[email protected]>

* Work around cpython `rootpath` being unavailable on Windows

For some reason `dependencies["cpython"].cpp_info.rootpath` is currently
unavailable on Windows (works on Linux), so resort to using the
deprecated `deps_cpp_info` instead, on the assumption that this is a
Conan bug/omission on the road to 2.0.

* Fix Python executable path on Windows

Either the backslashes need to be escaped, or we can just use a posix
path. Otherwise CMake complains about unknown escape sequences.

* Fix Python library path for Windows

The library in `cpp_info.libs` is the library name, not the library
filename, so we must add the file extension.

We must also then convert the path to posix to get rid of the
backslashes.

* Work around cpython package static build missing import library

In the static build of cpython the `python39.lib` file is,
understandably, a static library rather than a Windows dll import
library.

When building a Python extension module we require the import library,
since the extension module itself should not statically link the whole
of Python(!).

So switch to the `shared` variant of cpython for Windows.

* Rename option from `without_python` to `with_python`

Signed-off-by: David Feltell <[email protected]>

* Move enforcement of `cpython:shared=True` to `configure` method

Signed-off-by: David Feltell <[email protected]>

* Bump OpenAssetIO version to alpha.8

Signed-off-by: David Feltell <[email protected]>

* Bump required CMake version to latest

Dependencies may want a newer version than 3.21, but if we're the root
recipe then we'll override it.

Signed-off-by: David Feltell <[email protected]>

* Bump require pybind11 version

New patch version now available.

Signed-off-by: David Feltell <[email protected]>

* Simplify CXX11_ABI CMake option computation

Slight change of behaviour, since the option will now always be set, but
doesn't do any harm.

Signed-off-by: David Feltell <[email protected]>

* Remove erroneous `cpp_info.libs` entry

The list of libraries is broken down by component, so just blank the
top-level list.

Signed-off-by: David Feltell <[email protected]>

* Use `package_folder` when computing paths

Use the more modern `package_folder` property, rather than `rootpath`.

Signed-off-by: David Feltell <[email protected]>

* Add TODO re. `cpython` recipe V2 incompatibility

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

* Add `validate` error if MSVC and not `cpython:shared=True`

Although we force `cpython:shared=True` in `configure()`, that may be
overridden by another package in the dependency tree.

Signed-off-by: David Feltell <[email protected]>

* Update `validate()` to match updated package template

i.e. `self.info.settings` -> `self.settings`, to match
`docs/package_templates/cmake_package/conanfile.py`

Signed-off-by: David Feltell <[email protected]>

* Simplify `test_package`

Bundle the test project, rather than fetching from an external source,
and simplify its contents (e.g. no C API tests, assume correct Python
version, ...).

Signed-off-by: David Feltell <[email protected]>

* Bump OpenAssetIO version to 1.0.0-alpha.9

Another release happened in the meantime. Note that the (incomplete)
C API bindings are no longer built by default, so that Conan component
has been removed.

Signed-off-by: David Feltell <[email protected]>

* Remove unused `_stdcpp_library` now that C API is no longer built

Signed-off-by: David Feltell <[email protected]>

* Remove unused import

Signed-off-by: David Feltell <[email protected]>

* Simplify C++ test

Signed-off-by: David Feltell <[email protected]>

* Override ncurses version to one that has pre-built package available

Signed-off-by: David Feltell <[email protected]>

* Remove unnecessary `source_folder` argument to `get`

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

* Remove unnecessary `f`-string

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

* Remove unnecessary set of `generate`'s `scope` arg

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

* Remove `openssl` override now its no longer needed

Signed-off-by: David Feltell <[email protected]>

* Add missing `package_type` metadata

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

* Remove `python_version` option

If consumers want an OpenAssetIO built for a different Python version,
then they can override the `cpython` dependency to a different version.

Signed-off-by: David Feltell <[email protected]>

---------

Signed-off-by: David Feltell <[email protected]>
Co-authored-by: Chris Mc <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Waiting on tools or services belonging to the infra
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants