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

serd: add version 0.30.14 #11897

Merged
merged 13 commits into from
Aug 6, 2022
Merged

Conversation

bigerl
Copy link
Contributor

@bigerl bigerl commented Jul 24, 2022

Specify library name and version: serd/0.30.14

The build system changed to meson in this release. So a more thorough review is required.


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

@conan-center-bot

This comment has been minimized.

@bigerl
Copy link
Contributor Author

bigerl commented Jul 25, 2022

I would need help with reading the failed CI runs:

  • [linter] Conan v2 migration / Lint changed conanfile.py (v2 migration) (pull_request)
    • Is this something I should address? What alternative to conans.tools.cross_building should I use?
  • [required] serd/0.30.14@ Windows, Visual Studio
    • I do not understand why the build was canceled. The validation should only disable it for 0.30.12
  • [required] serd/0.30.14@ macOS, Clang (M1/arm64)
    • This line looks like some tool that is invoked seems not to be built for the building plattform:

       meson.build:4:0: ERROR: Could not invoke sanity test executable: [Errno 86] Bad CPU type in executable: '/Users/jenkins/w/prod/BuildSingleReference@2/.conan/data/serd/0.30.14/_/_/package/3558c54d13382045caaf9174821b7c2ba01fc8bc/build/meson-private/sanitycheckc.exe'.
      

      I am also not sure how to proceed here. My guess would be that it actually could be built for ARM MacOS.

A general question:

  • Does the meson integration for conan available in the conans.* packages support crossbuilding?

A closing comment on 0.30.12 vs 0.30.14. In 0.30.12, we disabled the build for VS and ARM MacOS because it was hard to figure out how to do it right with the customized WAF build system shipped with serd. With serd using meson now, I think it should be easy.

@uilianries
Copy link
Member

@bigerl Thank you for contribution!

Is this something I should address? What alternative to conans.tools.cross_building should I use?

It's not mandatory to pass/merge your PR, but it's well recommended. In some months, Conan 2.0 will be available and we will need to migrate all recipes, include this one. More information: https://docs.conan.io/en/latest/conan_v2.html

In short answer: Yes, please, replace it by https://docs.conan.io/en/latest/reference/conanfile/tools/build.html#conan-tools-build-cross-building

I do not understand why the build was canceled. The validation should only disable it for 0.30.12

Once a job fails, others are canceled to save CI time. It's a regular process are Conan Center Index needs to generated more than 100 packages for each new pull request.

This line looks like some tool that is invoked seems not to be built for the building plattform:

It's some Meson behavior with Serd project. I would say to read the Meson file provided by Serd and find someway to disable that validation.

Does the meson integration for conan available in the conans.* packages support crossbuilding?

Yes, CCI has around 30 recipes using Meson. Still, cross-building is hard. I would suggest taking a look on https://github.com/conan-io/conan-center-index/search?q=Meson&type=code and check what other recipes are doing.

@SSE4
Copy link
Contributor

SSE4 commented Jul 26, 2022

  • Is this something I should address? What alternative to conans.tools.cross_building should I use?

a modern alternatiive is can_run

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@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 13 (a990b29ffcc5f44ce9c78b0bf51262f7d0bc9ec2):

  • serd/0.30.14@:
    All packages built successfully! (All logs)

    🔸 Informative: This recipe is not ready for Conan v2

    We have started the migration process to Conan v2 and exporting recipes successfully will be required in the future.
    This is just an informative note to gain awareness about the process, no need to take any action. The plan is to enforce smaller steps that are easier to fix and, eventually, this conan export step will work.
    See the recipe migration guide to know more about the changes required.

    ERROR: Error loading conanfile at '/home/conan/w/prod_cci_PR-11897/recipes/serd/all/conanfile.py': Unable to load conanfile in /home/conan/w/prod_cci_PR-11897/recipes/serd/all/conanfile.py
      File "<frozen importlib._bootstrap_external>", line 728, in exec_module
      File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
      File "/home/conan/w/prod_cci_PR-11897/recipes/serd/all/conanfile.py", line 5, in <module>
        from conans import ConanFile, tools, Meson
    ImportError: cannot import name 'ConanFile' from 'conans' (/opt/pyenv/versions/3.7.13/lib/python3.7/site-packages/conans/__init__.py)
    

Copy link
Contributor

@prince-chrismc prince-chrismc left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@conan-center-bot conan-center-bot merged commit cd7a689 into conan-io:master Aug 6, 2022
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.

None yet

5 participants