Re-enable avahi support for discovery

[awiouy]

This pull request allows to select whether discovery is built based on mdns (by default) or on avahi. I only streamlined Dockerfile and docker-build-avahi.sh. Many thanks to @shanemeagher, who did the heavy lifting.

[awiouy]

This works fine here: https://github.com/LibreELEC/LibreELEC.tv/pull/1622

[marcelveldt]

you should probably extend the code some more because when one doesn't provide any of the discovery options, the code won't build. If you add a few checks more it will also build without any discovery, for example on windows.

e.g.:

[cfg(any(feature = "with-rust-mdns", feature = "with-avahi"))]

use librespot::authentication::discovery::{discovery, DiscoveryStream};

[awiouy]

Thanks for the heads-up @marcelveldt My Rust is too basic to do it myself, though. If you create a commit in your github, I will add it to this pull request. Thanks in advance.

[shanemeagher]

@awiouy @marcelveldt I've updated my commits so that rust-mdns is used by default, even if --no-default-features is set. This keeps current behaviour for building librespot the same (also, my rust wasn't sufficient to build librespot with discovery disabled completely).

If someone want's to build librespot to use with avahi now, the with-avahi feature replaces rust-mdns with dns-sd.

I've also rebased on master and squashed it to one commit: https://github.com/shanemeagher/librespot/commit/6f515f42b7b9eb7ae3c896598cbf20bed769979f

[marcelveldt]

If I may chime in too, I don't think it's a great idea that it will force one of the two with no-default-features, in my case for example I'm also compiling for Windows and I need to comment out the dns stuff. So better to specify both of them as a separate feature but pick one as the default. And off course the changes in the code that it will only load (the correct) dns/discovery code if the feature is actually enabled.

I'm really busy atm otherwise I would have provided the PR

[plietar]

@marcelveldt currently if --no-default-features is used and neither is chosen explicitly, then it won't include any backend and build fine on Windows.

On the other hand, if both are enabled, it will fail to build, which is a problem to me

[shanemeagher]

@plietar finally getting a chance to revisit this.

Have made most of your suggested changes but have a (probably simple) problem in cargo.toml where I currently have the following features:

with-internal-mdns  = ["mdns"]
with-external-mdns  = ["dns-sd"]

If neither is specified, cargo tries to build with mdns but as mdns is only specified by with-internal-mdns, it fails. I could add with-internal-mdns to the default features but if --no-default-features is used, it will still fail.

Any suggestions?

[plietar]

@shanemeagher Did you use #[cfg(mdns)] around the extern crate mdns ?

[shanemeagher]

@plietar I've started a new PR https://github.com/plietar/librespot/pull/246 to push the code directly. This should address all your comments above.

[awiouy]

Feature taken over by #246. Closing.