Add ohPipeline

[DoomHammer]

Specify library name and version: ohpipeline/1.39.1000

This library is required to build audio products based on the OpenHome protocol.

---

• I've read the contributing guidelines.
• I've used a recent Conan client version close to the currently deployed.
• I've tried at least one configuration locally with the conan-center hook activated. <- I'm not sure how to make them work with Conan 2.0 as the 1.0 instructions don't work.

[DoomHammer]

@conan-io/barbarians any chance to use a more recent CMake?

Edit: it seems I don't have the proper ping abilities. @uilianries could you by any chance help here? 🙏

[uilianries]

Thank you for your PR!! Please, take a look on my review 😄

[uilianries]

This cmake file is huge. Please sent it to the upstream as pull request. I see the project is not active, but still, more users can use it and contribute with comments.

I see they offer a build support with waf.io, but not sure how good is it.

[DoomHammer]

I will send it. Unfortunately, I don't have good experience with sending patches upstream with this org but at least we'll have it there for documentation purposes :)

Do you think it'd make sense to split the main CMakeLists.txt into several smaller ones?

As for Waf: I tried using it form building a Conan package, but creating CMake turned out to be easier...

[uilianries]

Let's keep a single cmakefile for now. The current one is large, but simple to read. I totally understand trying to use another build tool that is not so supported by Conan makes life harder. There are other PRs that we accepted a custom cmake file to build the project.

[valgur]

I would recommend forking the library or at least creating a separate repo for the CMake project files.

[DoomHammer]

Ok, now I have some fPIC woes. Any idea how to approach those, @uilianries ?

[uilianries]

Ok, now I have some fPIC woes. Any idea how to approach those, @uilianries ?

I'm curious if it was because that workaround involving cmake_cxx_flags or is something related the the cmakefile. I'm not on my computer now, but I'll build it locally tomorrow.

[DoomHammer]

May this be caused by improper fPIC handling in ohNet?

[uilianries]

@DoomHammer Sorry the delay, I made a PR to your branch here: DoomHammer#1

Please, consider it as a fix for the current error. The fPIC error is occurring because all libraries are configured to be built as static, but Conan build both static and dynamic.

[DoomHammer]

@uilianries do you think ohNet library should also be updated regarding the static changes?

[DoomHammer]

It's very hacky but it works. @uilianries @memsharded any ideas if this could be improved?

[valgur]

Suggested change

	tc = CMakeToolchain(self)
	venv = VirtualBuildEnv(self)
	venv.generate()
	tc = CMakeToolchain(self)

Required for the CMake tool_requires.

[valgur]

@DoomHammer I opened a PR with all of my recipe improvement suggestions: DoomHammer#2

Thank you for moving the CMakeLists.txt into its own repo! The recipe looks much better now.

[DoomHammer]

@valgur do you see the entire error message? I cannot deduce what's the problem based on what's presented here and I also cannot reproduce the problem locally.

[AbrilRBS]

Hi @DoomHammer - I've restarted the PR's CI - We had an outagge with one of the OSx agents that caused some builds to fail without logs. It's now fixed so let's see if this builds now :) Thanks a lot for your patience!

[DoomHammer]

Hi @RubenRBS ! Unfortunately, it's the same :(

[valgur]

@DoomHammer The macOS Debug (but not Release) build fails with

In file included from /Users/jenkins/workspace/prod-v1/bsr@2/26380/effdd/.conan/data/ohpipeline/1.139.1000/_/_/build/2a909c6f087d7d10f9d1a2e759cb2f39f2dcdb77/src/OpenHome/Web/ConfigUi/ConfigUiMediaPlayer.cpp:8:
/Users/jenkins/workspace/prod-v1/bsr@2/26380/effdd/.conan/data/ohpipeline/1.139.1000/_/_/build/2a909c6f087d7d10f9d1a2e759cb2f39f2dcdb77/src/OpenHome/Av/Qobuz/Qobuz.h:15:10: fatal error: 'Generated/CpAvOpenhomeOrgPlaylist1.h' file not found
#include <Generated/CpAvOpenhomeOrgPlaylist1.h>
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.

If you can't figure out a fix or a workaround for this, you can probably just raise a ConanInvalidConfiguration exception for Macos + Debug.

[DoomHammer]

MacOS + Debug works for me locally albeit with a different clang version. But what we see here is a lack of file that's used both in Debug and Release. So this is extremely weird. A race condition maybe?

[DoomHammer]

Which CMake generator does the CI use? Is it Makefile or Ninja or something else?

[valgur]

It uses the default CMake generator - Unix Makefiles.
Did you try with Conan v1 as well?

[DoomHammer]

Yes, Conan v1 seems to work for me. I found a possible fix, though, so I'm trying it now.

[DoomHammer]

Nope, I only made that worse...

[conan-center-bot]

Conan v1 pipeline ✔️

All green in build 6 (4ed0c7f7ed9316121d3e5c86697918410bd7e382):

• 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 6 (4ed0c7f7ed9316121d3e5c86697918410bd7e382):

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

[DoomHammer]

OMG, we made it 😱

[valgur]

Please submit it to the upstream project as well, if you already haven't.

[DoomHammer]

The reason CMake ended up here is the same as the reason the patch is here and the reason why I made my fork. The upstream does not accept patches.

[DoomHammer]

Exhibit A: openhome/ohPipeline#8

[DoomHammer]

Also: I see this has been fixed in the more recent versions, which I want to handle after this one is merged. It's been lingering way to long already :P

[valgur]

Nice! Thanks!

[DoomHammer]

Can we get that merged, then?

[danimtb]

I know I might be a bit late for the review but think we should not incorporate new build systems to a 3rd party library as there might be errors or subtle behavior changes for the end user. However, I understand the effort here and the usage of an external fork-provided CMakeLists. For future iterations of this recipe, let's try to use the original waf build system and stick to only the sources and instructions provided by the library. Thanks!

[DoomHammer]

Making the original work with Conan probably requires more code than just using CMake but ends up being less flexible...

[danimtb]

thanks for the input. Yes, I understand, but maintaining a different build system as the library evolves will be an issue for the recipe here in conan-center as well. We have to take care of that as maintenance of that could be a nightmare as well.

[DoomHammer]

True. But it would be nightmare mostly for me, either way ;) The current version of the library doesn't even compile unless you have non-free SDK, so I'll need to patch it to handle this as an option.