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

Build with CMake on unix #128

Merged
merged 19 commits into from
Sep 6, 2022
Merged

Conversation

h-vetinari
Copy link
Member

As #127:

Testing things like grpc through the examples provided upstream needs find_package(protobuf) to run, which fails on unix with

CMake Error at /home/conda/feedstock_root/build_artifacts/grpc-split_1662220567868/test_tmp/examples/cpp/cmake/common.cmake:101 (find_package):
  Could not find a package configuration file provided by "Protobuf" with any
  of the following names:

    ProtobufConfig.cmake
    protobuf-config.cmake

  Add the installation prefix of "Protobuf" to CMAKE_PREFIX_PATH or set
  "Protobuf_DIR" to a directory containing one of the above files.  If
  "Protobuf" provides a separate development package or SDK, be sure it has
  been installed.

because the builds here are done with autotools.

Build with cmake to be able to run better testing in grpc-cpp feedstock. Also:

  • normalize cmake module location on windows
  • unify build scripts
  • improve testing

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@h-vetinari
Copy link
Member Author

This PR is ready, but it's better to direct review efforts at #127 to avoid duplicated efforts.

@peastman
Copy link

peastman commented Sep 6, 2022

Here's the full package plan from conda create.

## Package Plan ##

  environment location: /Users/peastman/miniconda3/envs/test

  added / updated specs:
    - python=3.9
    - pytorch


The following packages will be downloaded:

    package                    |            build
    ---------------------------|-----------------
    libcxx-14.0.6              |       hccf4f1f_0         1.3 MB  conda-forge
    libgfortran-5.0.0          |10_4_0_h97931a8_25         146 KB  conda-forge
    libprotobuf-3.20.1         |       hbc0c0cd_2         2.2 MB  conda-forge
    sleef-3.5.1                |       h6db0672_2         1.0 MB  conda-forge
    ------------------------------------------------------------
                                           Total:         4.7 MB

The following NEW packages will be INSTALLED:

  bzip2              conda-forge/osx-64::bzip2-1.0.8-h0d85af4_4
  ca-certificates    conda-forge/osx-64::ca-certificates-2022.6.15-h033912b_0
  cffi               conda-forge/osx-64::cffi-1.15.1-py39hae9ecf2_0
  libblas            conda-forge/osx-64::libblas-3.9.0-16_osx64_openblas
  libcblas           conda-forge/osx-64::libcblas-3.9.0-16_osx64_openblas
  libcxx             conda-forge/osx-64::libcxx-14.0.6-hccf4f1f_0
  libffi             conda-forge/osx-64::libffi-3.4.2-h0d85af4_5
  libgfortran        conda-forge/osx-64::libgfortran-5.0.0-10_4_0_h97931a8_25
  libgfortran5       conda-forge/osx-64::libgfortran5-11.3.0-h082f757_25
  liblapack          conda-forge/osx-64::liblapack-3.9.0-16_osx64_openblas
  libopenblas        conda-forge/osx-64::libopenblas-0.3.21-openmp_h429af6e_3
  libprotobuf        conda-forge/osx-64::libprotobuf-3.20.1-hbc0c0cd_2
  libsqlite          conda-forge/osx-64::libsqlite-3.39.3-ha978bb4_0
  libzlib            conda-forge/osx-64::libzlib-1.2.12-hfe4f2af_2
  llvm-openmp        conda-forge/osx-64::llvm-openmp-14.0.4-ha654fa7_0
  mkl                conda-forge/osx-64::mkl-2022.1.0-h860c996_928
  ncurses            conda-forge/osx-64::ncurses-6.3-h96cf925_1
  ninja              conda-forge/osx-64::ninja-1.11.0-h1b54a9f_0
  numpy              conda-forge/osx-64::numpy-1.23.2-py39h62c883e_0
  openssl            conda-forge/osx-64::openssl-3.0.5-hb81d4ab_1
  pip                conda-forge/noarch::pip-22.2.2-pyhd8ed1ab_0
  pycparser          conda-forge/noarch::pycparser-2.21-pyhd8ed1ab_0
  python             conda-forge/osx-64::python-3.9.13-hf8d34f4_0_cpython
  python_abi         conda-forge/osx-64::python_abi-3.9-2_cp39
  pytorch            conda-forge/osx-64::pytorch-1.12.1-cpu_py39h9b0ea23_0
  readline           conda-forge/osx-64::readline-8.1.2-h3899abd_0
  setuptools         conda-forge/noarch::setuptools-65.3.0-pyhd8ed1ab_1
  sleef              conda-forge/osx-64::sleef-3.5.1-h6db0672_2
  sqlite             conda-forge/osx-64::sqlite-3.39.3-h9ae0607_0
  tbb                conda-forge/osx-64::tbb-2021.5.0-hbb4e6a2_1
  tk                 conda-forge/osx-64::tk-8.6.12-h5dbffcc_0
  typing_extensions  conda-forge/noarch::typing_extensions-4.3.0-pyha770c72_0
  tzdata             conda-forge/noarch::tzdata-2022c-h191b570_0
  wheel              conda-forge/noarch::wheel-0.37.1-pyhd8ed1ab_0
  xz                 conda-forge/osx-64::xz-5.2.6-h775f41a_0

@jrbourbeau
Copy link
Member

Just noting that we're seeing something similar over in Dask's macOS Python 3.9 CI build when attempting to import pyarrow

E   ImportError: dlopen(/Users/runner/miniconda3/envs/test-environment/lib/python3.9/site-packages/pyarrow/lib.cpython-39-darwin.so, 2): Library not loaded: @rpath/libprotobuf.31.dylib
E     Referenced from: /Users/runner/miniconda3/envs/test-environment/lib/liborc.dylib
E     Reason: Incompatible library version: liborc.dylib requires version 32.0.0 or later, but libprotobuf.31.dylib provides version 31.0.0
Full traceback:
____________________ test_chunksize[fastparquet-1024-True] _____________________
[gw3] darwin -- Python 3.9.13 /Users/runner/miniconda3/envs/test-environment/bin/python

mod_name = 'pyarrow', error_msg = '`pyarrow` not installed'

    def import_required(mod_name, error_msg):
        """Attempt to import a required dependency.
    
        Raises a RuntimeError if the requested module is not available.
        """
        try:
>           return import_module(mod_name)

dask/utils.py:194: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

name = 'pyarrow', package = None

    def import_module(name, package=None):
        """Import a module.
    
        The 'package' argument is required when performing a relative import. It
        specifies the package to use as the anchor point from which to resolve the
        relative import to an absolute import.
    
        """
        level = 0
        if name.startswith('.'):
            if not package:
                msg = ("the 'package' argument is required to perform a relative "
                       "import for {!r}")
                raise TypeError(msg.format(name))
            for character in name:
                if character != '.':
                    break
                level += 1
>       return _bootstrap._gcd_import(name[level:], package, level)

../../../miniconda3/envs/test-environment/lib/python3.9/importlib/__init__.py:127: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

name = 'pyarrow', package = None, level = 0

>   ???

<frozen importlib._bootstrap>:1030: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

name = 'pyarrow', import_ = <function _gcd_import at 0x10473c3a0>

>   ???

<frozen importlib._bootstrap>:1007: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

name = 'pyarrow', import_ = <function _gcd_import at 0x10473c3a0>

>   ???

<frozen importlib._bootstrap>:986: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

spec = ModuleSpec(name='pyarrow', loader=<_frozen_importlib_external.SourceFileLoader object at 0x14aaa02b0>, origin='/Users/...py', submodule_search_locations=['/Users/runner/miniconda3/envs/test-environment/lib/python3.9/site-packages/pyarrow'])

>   ???

<frozen importlib._bootstrap>:680: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <_frozen_importlib_external.SourceFileLoader object at 0x14aaa02b0>
module = <module 'pyarrow' from '/Users/runner/miniconda3/envs/test-environment/lib/python3.9/site-packages/pyarrow/__init__.py'>

>   ???

<frozen importlib._bootstrap_external>:850: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

f = <built-in function exec>
args = (<code object <module> at 0x14a983ea0, file "/Users/runner/miniconda3/envs/test-environment/lib/python3.9/site-package...n', '__file__': '/Users/runner/miniconda3/envs/test-environment/lib/python3.9/site-packages/pyarrow/__init__.py', ...})
kwds = {}

>   ???

<frozen importlib._bootstrap>:228: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

    """
    PyArrow is the python implementation of Apache Arrow.
    
    Apache Arrow is a cross-language development platform for in-memory data.
    It specifies a standardized language-independent columnar memory format for
    flat and hierarchical data, organized for efficient analytic operations on
    modern hardware. It also provides computational libraries and zero-copy
    streaming messaging and interprocess communication.
    
    For more information see the official page at https://arrow.apache.org/
    """
    
    import gc as _gc
    import importlib as _importlib
    import os as _os
    import platform as _platform
    import sys as _sys
    import warnings as _warnings
    
    try:
        from ._generated_version import version as __version__
    except ImportError:
        # Package is not installed, parse git tag at runtime
        try:
            import setuptools_scm
            # Code duplicated from setup.py to avoid a dependency on each other
    
            def parse_git(root, **kwargs):
                """
                Parse function for setuptools_scm that ignores tags for non-C++
                subprojects, e.g. apache-arrow-js-XXX tags.
                """
                from setuptools_scm.git import parse
                kwargs['describe_command'] = \
                    "git describe --dirty --tags --long --match 'apache-arrow-[0-9].*'"
                return parse(root, **kwargs)
            __version__ = setuptools_scm.get_version('../',
                                                     parse=parse_git)
        except ImportError:
            __version__ = None
    
    # ARROW-8684: Disable GC while initializing Cython extension module,
    # to workaround Cython bug in https://github.com/cython/cython/issues/3603
    _gc_enabled = _gc.isenabled()
    _gc.disable()
>   import pyarrow.lib as _lib
E   ImportError: dlopen(/Users/runner/miniconda3/envs/test-environment/lib/python3.9/site-packages/pyarrow/lib.cpython-39-darwin.so, 2): Library not loaded: @rpath/libprotobuf.31.dylib
E     Referenced from: /Users/runner/miniconda3/envs/test-environment/lib/liborc.dylib
E     Reason: Incompatible library version: liborc.dylib requires version 32.0.0 or later, but libprotobuf.31.dylib provides version 31.0.0

../../../miniconda3/envs/test-environment/lib/python3.9/site-packages/pyarrow/__init__.py:65: ImportError

The above exception was the direct cause of the following exception:

tmpdir = local('/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/pytest-of-runner/pytest-0/popen-gw3/test_chunksize_fastparquet_1020')
chunksize = 1024, engine = 'fastparquet', metadata = True

    @pytest.mark.parametrize("metadata", [True, False])
    @pytest.mark.parametrize("chunksize", [None, 1024, 4096, "1MiB"])
    def test_chunksize(tmpdir, chunksize, engine, metadata):
        nparts = 2
        df_size = 100
        row_group_size = 5
    
        df = pd.DataFrame(
            {
                "a": np.random.choice(["apple", "banana", "carrot"], size=df_size),
                "b": np.random.random(size=df_size),
                "c": np.random.randint(1, 5, size=df_size),
                "index": np.arange(0, df_size),
            }
        ).set_index("index")
    
        ddf1 = dd.from_pandas(df, npartitions=nparts)
>       ddf1.to_parquet(
            str(tmpdir),
            engine="pyarrow",
            row_group_size=row_group_size,
            write_metadata_file=metadata,
        )

dask/dataframe/io/tests/test_parquet.py:2923: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
dask/dataframe/core.py:5145: in to_parquet
    return to_parquet(self, path, *args, **kwargs)
dask/dataframe/io/parquet/core.py:764: in to_parquet
    engine = get_engine(engine)
dask/dataframe/io/parquet/core.py:1153: in get_engine
    pa = import_required("pyarrow", "`pyarrow` not installed")
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

mod_name = 'pyarrow', error_msg = '`pyarrow` not installed'

    def import_required(mod_name, error_msg):
        """Attempt to import a required dependency.
    
        Raises a RuntimeError if the requested module is not available.
        """
        try:
            return import_module(mod_name)
        except ImportError as e:
>           raise RuntimeError(error_msg) from e
E           RuntimeError: `pyarrow` not installed

dask/utils.py:196: RuntimeError

@h-vetinari
Copy link
Member Author

h-vetinari commented Sep 7, 2022

Argh, this might be that cmake sets different so versions than the autotools builds.

There are some differences, but the SO version is the same as before, which makes the error quite bizarre, because the pytorch build logs say:

INFO (pytorch,lib/python3.10/site-packages/torch/lib/libtorch_python.dylib): Needed DSO lib/libprotobuf.31.dylib found in conda-forge::libprotobuf-3.20.1-h2292cb8_0

which directly contradicts the error being reported:

Reason: Incompatible library version: libtorch_python.dylib requires version 32.0.0 or later, but libprotobuf.31.dylib provides version 31.0.0

I'm really baffled by this, but I could vaguely imagine that, not having cmake metadata, the build system does something weird and encodes a requirement for "+1", and then ends up failing once that guess is no more necessary.

CC @isuruf, if you any idea what might be happening here.

@isuruf
Copy link
Member

isuruf commented Sep 7, 2022

Switching from autotools to cmake is not ABI compatible most of the time, unless the upstream maintainers care about it (which is not the case most of the time). I wrote a checklist of things to do in https://conda-forge.org/docs/maintainer/knowledge_base.html#moving-from-an-autotools-build-to-a-cmake-build

I think you should mark the latest builds as broken ASAP.

@h-vetinari
Copy link
Member Author

I think you should mark the latest builds as broken ASAP.

See conda-forge/admin-requests#480 (currently only for osx, because further up it was explicitly mentioned that "This only happens on x86, not ARM." - happy to expand it though, if you think that's prudent)

@isuruf
Copy link
Member

isuruf commented Sep 7, 2022

I think it's better to mark all versions as broken, do the checklist and then re-enable cmake builds.

@h-vetinari
Copy link
Member Author

I think it's better to mark all versions as broken, do the checklist and then re-enable cmake builds.

Expanded conda-forge/admin-requests#480.

@isuruf
Copy link
Member

isuruf commented Sep 7, 2022

CMake was introduced in #47 and immediately reverted because of the same issue. Please have a look at #49 for a previous attempt at fixing it.

@h-vetinari
Copy link
Member Author

CMake was introduced in #47 and immediately reverted because of the same issue. Please have a look at #49 for a previous attempt at fixing it.

Thanks for the context! I'll see if I can use that to fix it, or otherwise we could just change over at 3.22. In the meantime, I'm reverting this in #129, so that the branches remain publishable.

@traversaro
Copy link
Contributor

I am not sure what is the reason and I can't investigate at the moment, but just FYI it seems that this PR also changed something on the Windows side leading do regressions, unless I am wrong. See robotology/robotology-superbuild#1251 .

@h-vetinari
Copy link
Member Author

I am not sure what is the reason and I can't investigate at the moment, but just FYI it seems that this PR also changed something on the Windows side leading do regressions, unless I am wrong. See robotology/robotology-superbuild#1251 .

Windows used to be built with cmake already, the only change there was to make the static builds depend on the shared ones (as on unix), and correspondingly rename the import libraries from libprotobuf.lib to libprotobuf-import.lib (so as not to collide with the libprotobuf.lib static ones). But the CMake metadata for these things should have all the required information, so I'm surprised to see the somewhat schizophrenic error

  Could NOT find Protobuf (missing: Protobuf_LIBRARIES) (found version "3.21.5")

Are you using libprotobuf or libprotobuf-static?

@traversaro
Copy link
Contributor

Are you using libprotobuf or libprotobuf-static?

The error is related to project that do not use/link protobuf at all, that is just a (configuration) error of the line:

find_package(Protobuf REQUIRED)

in https://github.com/osrf/gazebo/blob/f6d2b940bfe47d9d7841bd793d844e28eb1fc75d/cmake/gazebo-config.cmake.in#L169 .

@traversaro
Copy link
Contributor

But the CMake metadata for these things should have all the required information,

The problem is that find_package(Protobuf REQUIRED) calls do not use the protobuf-config.cmake file installed by CMake, but rather the FindProtobuf.cmake file of CMake (see https://github.com/Kitware/CMake/blob/v3.24.1/Modules/FindProtobuf.cmake), that do not call find_package(Protobuf CONFIG) instead.

@h-vetinari
Copy link
Member Author

The problem is that find_package(Protobuf REQUIRED) calls do not use the protobuf-config.cmake file installed by CMake, but rather the FindProtobuf.cmake file of CMake (see https://github.com/Kitware/CMake/blob/v3.24.1/Modules/FindProtobuf.cmake), that do not call find_package(Protobuf CONFIG) instead.

🤦
Why would CMake insist on its custom logic if there's package metadata created by CMake available...?

@h-vetinari
Copy link
Member Author

Well, it could be solved by adding "-import.lib" to CMAKE_FIND_LIBRARY_SUFFIXES, but I guess with the way CMake works, windows isn't really ready for having the static lib depend on the shared one.

@traversaro
Copy link
Contributor

Are you using libprotobuf or libprotobuf-static?

The error is related to project that do not use/link protobuf at all, that is just a (configuration) error of the line:

find_package(Protobuf REQUIRED)

in https://github.com/osrf/gazebo/blob/f6d2b940bfe47d9d7841bd793d844e28eb1fc75d/cmake/gazebo-config.cmake.in#L169 .

Ah sorry, I get your question now, libprotobuf and libprotobuf-static are conda packages. In that case, I had just libprotobuf installed.

@traversaro
Copy link
Contributor

traversaro commented Sep 7, 2022

The problem is that find_package(Protobuf REQUIRED) calls do not use the protobuf-config.cmake file installed by CMake, but rather the FindProtobuf.cmake file of CMake (see https://github.com/Kitware/CMake/blob/v3.24.1/Modules/FindProtobuf.cmake), that do not call find_package(Protobuf CONFIG) instead.

🤦 Why would CMake insist on its custom logic if there's package metadata created by CMake available...?

I guess it is just an historical legacy of Find<PackageName>.cmake having precendence over <PackageName>-config.cmake, but indeed I do not know the original rationale for that, I guess it was due to the fact that the two files may have used different variable names and/or logic, and Find<PackageName>.cmake I guess were introduced before.

Historically, this counterintuitive behavior has been handled by explicitly adding a find_package(<PackageName> CONFIG) in the Find<PackageName>.cmake file, together with a logic to convert <PackageName>-config.cmake variables/targets to Find<PackageName>.cmake variables/targets, see for example https://github.com/Kitware/CMake/blob/v3.24.1/Modules/FindGLEW.cmake#L67 . An issue for doing the same for FindProtobuf is opened at https://gitlab.kitware.com/cmake/cmake/-/issues/16063 , but I guess no one ever tried to solve that.

As an example of tricky incompatibilities that are different between FindProtobuf.cmake and protobuf-config.cmake, the whole logic that permits to do cross-compilation of projects that use protoc, i.e. setting Protobuf_PROTOC_EXECUTABLE to the build protoc instead of the host one as done in https://github.com/conda-forge/libignition-msgs1-feedstock/blob/40aad6e226cf0e8b8171d4c47c271dcecd83fe10/recipe/build.sh#L33, is possible in FindProtobuf.cmake, but not supported in protobuf-config.cmake. See also https://gitlab.kitware.com/cmake/cmake/-/issues/21228 for other similar problems.

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.

7 participants