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

Update CMakeFile for ohPipeline #23788

Merged

Conversation

DoomHammer
Copy link
Contributor

CMAKE_BUILD_TYPE is apparently not set in Windows builds. The new approach should handle this.


@conan-center-bot

This comment has been minimized.

@DoomHammer
Copy link
Contributor Author

BTW: if there is a better way to handle the ServiceGen path handover than this: https://github.com/conan-io/conan-center-index/blob/master/recipes/ohnet/all/conanfile.py#L113 I'd be happy to learn. Somehow this feels extremely hacky.

@@ -4,8 +4,8 @@ sources:
url: "https://github.com/openhome/ohPipeline/archive/refs/tags/ohMediaPlayer_1.139.1000.tar.gz"
sha256: "070eac6d974bbe520d0b292df1ca4a72160a2b607a60dabdaa0971c3ef96e5e4"
cmake:
url: "https://raw.githubusercontent.com/merakiacoustic/ohPipeline/ohMediaPlayer_1.139.1000_meraki2/CMakeLists.txt"
sha256: "f9e11d4adb8426a8375dae02e3b641d66efc6ef3abd6f96cb81f6e85352083a3"
url: "https://raw.githubusercontent.com/merakiacoustic/ohPipeline/ohMediaPlayer_1.139.1000_meraki3/CMakeLists.txt"
Copy link
Contributor

Choose a reason for hiding this comment

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

Needing to maintain a completely custom build system for a project is already a nightmare, but even more so when externalizing the scripts. I am not sure why this was accepted.

If it is too big to be included in CCI's repository, it is too big to be maintained. Can we please integrate it back here or have a general discussion about this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've also created a separate PR that tries to work with existing build system but it fails in ways I can't reproduce :(

Copy link
Member

Choose a reason for hiding this comment

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

Agree this should be done, but I know it can be complicated too.
So approving this fix in the meantime, thanks very much for contributing it @DoomHammer !

@memsharded memsharded self-assigned this Jun 11, 2024
@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

Conan v1 pipeline ✔️

All green in build 5 (285a7bf8e8f596bb59a4c9b241d3344748eb66ff):

  • ohpipeline/1.139.1000:
    All packages built successfully! (All logs)

Conan v2 pipeline ✔️

Note: Conan v2 builds are now mandatory. Please read our discussion about it.

All green in build 5 (285a7bf8e8f596bb59a4c9b241d3344748eb66ff):

  • ohpipeline/1.139.1000:
    All packages built successfully! (All logs)

@conan-center-bot conan-center-bot merged commit f535c16 into conan-io:master Jun 11, 2024
13 checks passed
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.

5 participants