Skip to content

First attempt at Conan package for system-installed Avahi Apple Bonjour compatibility library #6928

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

Closed
wants to merge 7 commits into from

Conversation

garethsb
Copy link
Contributor

@garethsb garethsb commented Aug 19, 2021

Specify library name and version: avahi-system/system avahi-mdnsresponder/system

Related to #6891, see #6891 (comment).

I have named this avahi-system named this avahi-mdnsresponder and used version="system" since I think it would be possible to make other packages packages for other components of Avahi and other recipe versions that built from the source at https://github.com/lathiat/avahi, but I believe most users will want to use the installer package for their system.

I have tested the apt install locally, the yum, pacman and zypper package lists are my best guess only.

I'm not sure precisely what needs to go in the package_info, e.g. system_libs, includedirs, libdirs.

Presumably this will need white-listing in order to build in CI.
@uilianries, @madebr, @prince-chrismc, thanks for your help so far!
I will submit a second PR for building mDNSResponder from the Apple tarballs.


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

…iscovery for Linux using mDNS/DNS-SD - compatible with Bonjour
@conan-center-bot

This comment has been minimized.

prince-chrismc
prince-chrismc previously approved these changes Aug 19, 2021
…so that "from source" versions can (later) have a different recipe
@conan-center-bot

This comment has been minimized.

prince-chrismc
prince-chrismc previously approved these changes Aug 19, 2021
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@SSE4 SSE4 requested a review from uilianries August 20, 2021 06:52
@ericLemanissier
Copy link
Contributor

ericLemanissier commented Aug 20, 2021

FYI, this PR fails on opensuse tumbleweed: https://github.com/bincrafters/system-packages-checks/runs/3379541409?check_suite_focus=true#step:6:330, leap: https://github.com/bincrafters/system-packages-checks/runs/3379541449?check_suite_focus=true#step:6:536. Both these errors should be solved by the code suggestion below (replacing libdns_sd with avahi-compat-mDNSResponder-devel).
there is another error on centos8: https://github.com/bincrafters/system-packages-checks/runs/3380619317?check_suite_focus=true#step:6:343 Error: Unable to find a match: avahi-tools. This does not seem to be available : https://forums.centos.org/viewtopic.php?t=71858. Do you think we can simply remove it ?

@Croydon
Copy link
Contributor

Croydon commented Aug 20, 2021

but I believe most users will want to use the installer package for their system.

Could you please explain your reasoning why your think that most users want to get it from their system package manager? Do you fear incompatibilities?

… Bonjour compatibility library, and use the right package name for openSUSE (thanks, @ericLemanissier)
@conan-center-bot

This comment has been minimized.

@garethsb
Copy link
Contributor Author

but I believe most users will want to use the installer package for their system.

Could you please explain your reasoning why your think that most users want to get it from their system package manager? Do you fear incompatibilities?

I'm not sure how closely coupled the daemon and client shared libraries are. Maybe I'm being overly cautious. :-/

@ericLemanissier
Copy link
Contributor

ericLemanissier commented Aug 20, 2021

on the distro side, it's now fine everywhere.
Are you sure that avahi/system is the proper name for this package ? Maybe we should be a little more specific, in order to be able to have packages for other avahi-foo packages built from https://github.com/lathiat/avahi ?
And as @Croydon said, a rationale is needed regarding the compatibility between system installed avahi and this library.

@garethsb
Copy link
Contributor Author

garethsb commented Aug 20, 2021

on the distro side, it's now fine everywhere.
Are you sure that avahi/system is the proper name for this package ? Maybe we should be a little more specific, in order to be able to have packages for other avahi-foo packages built from https://github.com/lathiat/avahi ?

avahi-libdns_sd or avahi-mdnsresponder (keeping version="system" to allow for future building from source) perhaps?
And add a provides as @prince-chrismc suggested?

…ahi to provide the Apple Bonjour compatibility library
garethsb added a commit to garethsb/conan-center-index that referenced this pull request Aug 20, 2021
@prince-chrismc
Copy link
Contributor

prince-chrismc commented Aug 20, 2021

zap

@conan-center-bot

This comment has been minimized.

garethsb added a commit to garethsb/conan-center-index that referenced this pull request Aug 20, 2021
Comment on lines +2 to +3
"system":
folder: system
Copy link
Contributor

@SpaceIm SpaceIm Aug 20, 2021

Choose a reason for hiding this comment

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

As @Croydon has pointing out, is it the good approach? I'm not comfortable with adding a new system library while source code is available.

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 started work on an avahi recipe from the source package. The configure script has 25 --enable-xxx options and a further 10 arguments for those options. It will need a new recipe for libdaemon for sure, and several other new recipes if all the options are ultimately to be supported.

Copy link
Contributor

Choose a reason for hiding this comment

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

Consumers of avahi are already using the system package manager, which goes the extra mile to setup the startup scripts for the daemon. Which could be added as a deploy step in downstream recipes to be installed but I am not sure how that would work in CCI.

Even if we build from source we won't be able to correctly install the daemon for that platform.
This give Consumers the option to build and deploy against a target platform and not have to re-invite too many things.

Having the recipe would be a huge advantage to "us" because it blends so nicely with the solutions we've already deployed and are working for end-users.


We all agree from source is ideal but in some cases, id argue like this, offer a few key advantages.
This does not prevent us from adding it in the future.

@conan-center-bot
Copy link
Contributor

All green in build 7 (2c49a8594e3ec9a1fb931078d34835104c18dbc8):

  • avahi-mdnsresponder/system@:
    All packages built successfully! (All logs)

This was referenced Aug 24, 2021
@garethsb
Copy link
Contributor Author

Closing in favour of #7049 which builds Avahi from source.

@garethsb garethsb closed this Aug 27, 2021
conan-center-bot pushed a commit that referenced this pull request Sep 2, 2021
* nmos-cpp recipe files copied from sony/nmos-cpp#195

* Add config.yml for current tip of garethsb:library-cmake

* Attempt to install Avahi Apple Bonjour compatibility library on Linux

* Provide with_dnssd options based on #6928, #6930, #6937

* Fix typo in prev commit

* Simplify selection of appropriate requirements for DNS-SD

* For now, keep default option as mdnsresponder on Windows and Linux

* Leave most exceptions uncaught as they really aren't expected in the test_package (cf. the best practice shown in nmos-cpp-node example app) but do catch web::http::http_exception since the most likely cause of that is a port being in use or, on Windows with cpprestsdk built with its default settings, that the test_package wasn't run as administrator, which WinHTTP enforces to listen on the wildcard hostname.
Uncaught exceptions, via std::terminate, end up as STATUS_STACK_BUFFER_OVERRUN on Windows (see https://devblogs.microsoft.com/oldnewthing/20190108-00/?p=100655), which Conan reports as "ConanException: Error 3221226505 while executing bin\test_package", which might take one a while to understand, if one didn't already know all of this...

* Update to head revision of garethsb:library-cmake and latest mdnsresponder (#6930) and avahi (#7049) from-source recipes, since those seem more likely to be approved on CCI

* Better comment

* Switch to upstream sony/nmos-cpp
(sony/nmos-cpp#195 was merged to master)

* self.options.get_safe("with_dnssd")

* For now, to build on CCI, don't require the opt-in patches

* Build requires CMake 3.17

* Hopefully using short paths will resolve "fatal error C1083: Cannot open compiler generated file: '': Invalid argument" on some Windows builds on CCI

* Test package requires C++11

* int64_t should work everywhere... I think

* Test proposed upstream fix to make relocatable package per https://cmake.org/cmake/help/latest/manual/cmake-packages.7.html#creating-relocatable-packages

* Fix handling of ${_IMPORT_PREFIX}

* Fix handling of ${_IMPORT_PREFIX} and add tools.check_min_cppstd

* No version ranges

* Log all exceptions
to debug intermittent test_package failure on arch=x86_64, build_type=Release, compiler=Visual Studio, compiler.runtime=MT, compiler.version=14, os=Windows

* Additional warning

* Take upstream latest which merged the relocatable package fix (cf. bf1388a)

* Update conanfile.py

* Apply suggestions from code review (properly)

Co-authored-by: Anonymous Maarten <[email protected]>

* Apply suggestions from code review

Co-authored-by: Anonymous Maarten <[email protected]>

* Apply suggestions from code review

Co-authored-by: Anonymous Maarten <[email protected]>

* self.options["mdnsresponder"].with_opt_patches = True

* Apply suggestions from code review

Co-authored-by: Uilian Ries <[email protected]>

* cmake/3.21.1 conflicts with openssl/1.1.1l

* Update topics

* Try bump openssl/1.1.1l and cmake/3.21.2

* Take upstream latest, now built and tested with OpenSSL v1.1.1L

Co-authored-by: Anonymous Maarten <[email protected]>
Co-authored-by: Uilian Ries <[email protected]>
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