Add ohNetGenerated #24600
Add ohNetGenerated #24600
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Hi @DoomHammer thanks a lot for taking the time to add the library, we appreciate it! I have a few questions before approving, but overall it looks good :)
|
|
||
| def configure(self): | ||
| if self.options.shared: | ||
| raise ConanInvalidConfiguration(f"{self.ref} doesn't support shared builds") |
There was a problem hiding this comment.
Then this library should:
- Remove the
sharedoption - Set the
package_type = "static-libary"
| basic_layout(self, src_folder="src") | ||
|
|
||
| def requirements(self): | ||
| self.requires("ohnet/[>=1.36.5182]", transitive_headers=True, transitive_libs=True) |
There was a problem hiding this comment.
Again, for now, we need to pin this version. If there's a 1:1 correspondence we can map that, but version ranges should be treated with care in this context :)
| tc.generate() | ||
| else: | ||
| tc = AutotoolsToolchain(self) | ||
| tc.make_args.append("-j1") |
There was a problem hiding this comment.
Why is this necessary? Does the compilation fail if it tries to paralellize?
There was a problem hiding this comment.
I'm afraid so. Some files are being copied after they are read which results in file not found errors more or less.
| targets = "proxies devices" | ||
|
|
||
| include_dir = self.dependencies["ohnet"].cpp_info.includedirs[0] | ||
| lib_dir = self.dependencies["ohnet"].cpp_info.libdirs[0] |
There was a problem hiding this comment.
In the other PR you've extracted this into a function, I think it looks better like that, could you also do the same here? Thanks!
| args.extend([f"openhome_architecture={openhome_architecture}", f"detected_openhome_architecture={openhome_architecture}"]) | ||
| return args | ||
|
|
||
| def config_options(self): |
There was a problem hiding this comment.
What is the minimum C++ version that this library supports?
DoomHammer
force-pushed
the
add-ohnetgenerated
branch
from
e5b3484
to
36dd9ca
Compare
Conan v1 pipeline ✔️All green in build 9 (
Conan v2 pipeline ✔️
All green in build 9 (
|
Summary
Changes to recipe: ohNetGenerated/1.1.143
Motivation
Add ohNetGenerated
Details
Part of the OpenHome networked audio suite.