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

Fail to compile get_if_addrs #520

Closed
ghost opened this issue Aug 25, 2020 · 35 comments
Closed

Fail to compile get_if_addrs #520

ghost opened this issue Aug 25, 2020 · 35 comments

Comments

@ghost
Copy link

ghost commented Aug 25, 2020

Hi,
I'm trying to create a librespot package for NetBSD, as I want to add GUI support for Spotify.
librespot compiles fine as a dependency crate of ncspot but fails to build as a stand alone package.
Right now it fails to build with a Rust "out of scope" error when building get_if_addrs.

...
Compiling get_if_addrs v0.5.3
error[E0425]: cannot find function `do_broadcast` in this scope
   --> /usr/pkgsrc/wip/librespot/work/vendor/get_if_addrs-0.5.3/src/lib.rs:234:31
    |
234 |                         match do_broadcast(ifaddr) {
    |                               ^^^^^^^^^^^^ not found in this scope

error[E0425]: cannot find function `do_broadcast` in this scope
   --> /usr/pkgsrc/wip/librespot/work/vendor/get_if_addrs-0.5.3/src/lib.rs:253:31
    |
253 |                         match do_broadcast(ifaddr) {
    |                               ^^^^^^^^^^^^ not found in this scope

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0425`.
error: could not compile `get_if_addrs`.

Any suggestions?
Rust version is 1.45.2

Thanks!

@ghost
Copy link
Author

ghost commented Aug 27, 2020

Anyone with a clue on how to fix this?
I would really like to package librespot for NetBSD.

@ashthespy
Copy link
Member

ashthespy commented Aug 27, 2020

@voidpin Looking at that crate, there aren't any real new changes - but as a quick sanity check, are you able to build just that crate in isolation?

EDIT: Looking at this netbsd seems to be included, has something changed in the way Rust identifies NETBSD?

#[cfg(any(target_os = "linux", target_os = "android", target_os = "nacl"))]
pub fn do_broadcast(ifaddr: &ifaddrs) -> Option<IpAddr> {
    sockaddr::to_ipaddr(ifaddr.ifa_ifu)
}

#[cfg(any(
    target_os = "freebsd",
    target_os = "ios",
    target_os = "macos",
    target_os = "openbsd",
    target_os = "netbsd"
))]
pub fn do_broadcast(ifaddr: &ifaddrs) -> Option<IpAddr> {
    sockaddr::to_ipaddr(ifaddr.ifa_dstaddr)
}

@ghost
Copy link
Author

ghost commented Aug 27, 2020

Thx for your reply.
Usually, I don't package rust applications.
Most of my packages are Qt apps, i.e. C++

I have seen that crate is archived and also though about trying to compile alone.
The thing is librespot compiles just fine as a dependency of ncspot, which we (me and others) recently added to the pkg collection.

Again, I'm not used to package rust things, why isn't this crate needed when compiling ncspot but its needed for librespot?

I would like to commit this pkg and provide NetBSD with a GUI client, now that we have a TUI one.
I'll see tomorrow if I can get get_if_addrs to compile on its own.

@ashthespy
Copy link
Member

The thing is librespot compiles just fine as a dependency of ncspot, which we (me and others) recently added to the pkg collection.

Going by Cargo.toml from ncspot it doesn't use full librespot, but just a subset of it's crates. Notably - it doesn't use librespot-connect which is the crate with the dependency tee that uses get_if_addrs.

@ghost
Copy link
Author

ghost commented Aug 27, 2020

Thx! I'd missed that.
Hmm...
Maybe I could do the same and package that subset and still get what I need to get the GUI client running...

At least now I have a few options to look at.
Please don't close this issue just yet.

Wouldn't it be better to use an active crate instead of an archieved one? https://crates.io/crates/if-addrs

@ghost
Copy link
Author

ghost commented Aug 28, 2020

@ashthespy I can't seem to find which of the dependency crates of librespot-connect

[dependencies]
base64 = "0.10"
futures = "0.1"
hyper = "0.11"
log = "0.4"
num-bigint = "0.2"
protobuf = "~2.14.0"
rand = "0.7"
serde = "1.0"
serde_derive = "1.0"
serde_json = "1.0"
tokio-core = "0.1"
url = "1.7"
sha-1 = "0.8"
hmac = "0.7"
aes-ctr = "0.3"
block-modes = "0.3"

actually depends on get_if_addrs?
At least not by looking at crates.io

@ashthespy
Copy link
Member

ashthespy commented Aug 28, 2020

It is libmdns Btw, cargo tree is quite useful when you aren't familiar with the codebase..

Wouldn't it be better to use an active crate instead of an archieved one? https://crates.io/crates/if-addrs

That fork seems to fix issues with Android, nothing else? Were you able to compile it in your system?

@ghost
Copy link
Author

ghost commented Aug 28, 2020

It seems I need to get used to cargo :)
Thx for the hint about cargo tree.

I didn't find the time yet.

@ghost
Copy link
Author

ghost commented Sep 3, 2020

Non-fix

@snowkat
Copy link

snowkat commented Sep 16, 2020

@ashthespy That commit was made after the 0.5.3 release on crates.io, and it looks like that release is missing NetBSD support in lib.rs:

#[cfg(any(target_os = "linux", target_os = "android", target_os = "nacl"))]
fn do_broadcast(ifaddr: &posix_ifaddrs) -> Option<IpAddr> {
    sockaddr_to_ipaddr(ifaddr.ifa_ifu)
}

#[cfg(
    any(target_os = "freebsd", target_os = "ios", target_os = "macos", target_os = "openbsd")
)]
fn do_broadcast(ifaddr: &posix_ifaddrs) -> Option<IpAddr> {
    sockaddr_to_ipaddr(ifaddr.ifa_dstaddr)
}

Changing the dependencies in libmdns Cargo.toml to use get_if_addrs from git allowed everything to build on NetBSD:

get_if_addrs = { version = "0.5", git = "https://github.com/maidsafe-archive/get_if_addrs.git" }

I can submit an issue/PR in the libmdns repository for this if preferred.

@ghost ghost reopened this Sep 16, 2020
@ghost
Copy link
Author

ghost commented Sep 16, 2020

@snowkat This is awesome, thanks.
Issue reopen until this is fixed.
@ashthespy what do you suggest, I think a PR on libmdns would be the best solution but, I'm ok with other solutions.

@ghost
Copy link
Author

ghost commented Sep 16, 2020

@snowkat , @ashthespy Pull-request done, librespot-org/libmdns#16

@willstott101
Copy link
Contributor

Does https://crates.io/crates/if-addrs not include the patch from get_if_addrs repo? https://github.com/messense/if-addrs Seems to be ahead on commits. I'd rather libmdns depends on released crates where possible - as it has to be on crates.io itself.

@ghost
Copy link
Author

ghost commented Sep 16, 2020

Looking at if-addrs/src/ifaddrs_posix.rs the patch was merged on Feb 2019 and the latest release of if-addrs on crates.io is from Aug 2020. So yes, changing the dependency from get_if_addrs to if-addrs should also do it, unless I'm missing something.

@ghost
Copy link
Author

ghost commented Sep 16, 2020

@willstott101 Is this a better approach to you? And will it trigger a new release of librespot?

@willstott101
Copy link
Contributor

This is a much better solution for me. Nothing will happen automatically, but I can make the relevant releases of libmdns and PR to bump libmdns in librespot after work if you confirm the compile works on NetBSD by switching that dependency.

@ghost
Copy link
Author

ghost commented Sep 16, 2020

I won't be able to do it tonight but, I'll try to do it in the coming days and will let you know.

@snowkat if you have time for it please go ahead.
And once again thanks for finding the issue.

@snowkat
Copy link

snowkat commented Sep 16, 2020

I created a new PR (librespot-org/libmdns#17) with the stable-0.2.x branch as a base since it seems librespot still uses this version. Let me know if you'd prefer a PR be made against 0.4 instead, or alongside this.

willstott101 added a commit to librespot-org/libmdns that referenced this issue Sep 16, 2020
@willstott101
Copy link
Contributor

Hm, I knew these small dependencies would end up causing a load of hassle... messense/if-addrs#3

I'll try and get AppVeyor windows CI onto libmdns to catch this earlier, but luckily I was trying to publish the switch to if-addrs from my windows laptop and cargo caught me in my tracks when the compile failed...

If I can't get the windows build of if-addrs sorted in the next few days I'll look at going back to having the get-if-addrs code rolled into the libmdns codebase.

@ghost
Copy link
Author

ghost commented Sep 17, 2020

@snowkat Thanks for the PR, I had some personal issues to sort out and wasn't able to do it.

@willstott101 Thank you! Its indeed a problem relying on archived crates. Did it work with the Windows travis?
Also, thanks for the version bump.

@ashthespy Is it possible to get a point-release with libmdns-0.2.7? I'd really appreciate that, thx!

@willstott101
Copy link
Contributor

Windows travis is erroring due to more changes in if-addrs that weren't in get-if-addr the author of if-addrs has patched their lib already and added windows travis to their repo. But I haven't had a chance to test yet. Will want to make sure libmdns works within librespot on windows before actually releasing the point release.

@ghost
Copy link
Author

ghost commented Sep 17, 2020

I understand that, just let me know so I can update the package build, thx!

The same goes for @ashthespy regarding librespot

It feels close now :)

@ghost
Copy link
Author

ghost commented Sep 23, 2020

@willstott101 Hi, any news/progress on this? Did you fix the Windows issue or are you pulling-in get_if_addrs inside libmdns?

@willstott101
Copy link
Contributor

willstott101 commented Sep 24, 2020

Apologies, I've been a bit busy.

Just done some further testing and if_addrs works great (on windows) in rust stable, but not 1.40.0 which appears to be Librespot's minimum version...

I've opened a PR in if-addrs which resolves this, they were very responsive before so hopefully will be again.

I'll be sure to get this sorted one way or another this weekend. Since the if-addrs maintainer has been so responsive I'd like to keep using their project if possible 🤞

@ghost
Copy link
Author

ghost commented Sep 24, 2020

Thank you!
No need for apologise. Hopefully, the PR is merged.
Rust 1.40.0? I thought NetBSD's 1.45.2 was getting old :)

@ghost
Copy link
Author

ghost commented Oct 3, 2020

@willstott101 your PR with if-addrs has been merged.
Any chance of getting a release of libmdns and a subsequent release of librespot?
@ashthespy

Looking forward to add this to the NetBSD package collection.

@willstott101
Copy link
Contributor

I've made a PR into the dev branch of librespot. Is that the right place for you guys?

@ashthespy
Copy link
Member

Thanks!
@sashahilton00 and @awiouy are responsible for the crates.io release. I guess they will combine a few things before a release..

@ghost
Copy link
Author

ghost commented Oct 5, 2020

@willstott101 and @ashthespy
Awesome, looking forward to rebuild it and add support for several audio options.

sashahilton00 added a commit that referenced this issue Oct 6, 2020
Bump libmdns to 0.2.7 hopefully fixes: #520
@ghost
Copy link
Author

ghost commented Oct 7, 2020

@willstott101 , @ashthespy and @sashahilton00
I can confirm the package builds and installs without issues on NetBSD if building from the git-dev branch, so thank you.
@snowkat , thanks for finding the issue.

While waiting for a new librespot release, I'll see if I can get a git-taged package accepted into the main repo.

@ghost
Copy link
Author

ghost commented Oct 7, 2020

@willstott101
Copy link
Contributor

This never made it to master... will open a PR

@Johannesd3
Copy link
Contributor

0.1.5 contains only one single fix (Spotifyd/spotifyd#719)

@willstott101
Copy link
Contributor

Well opened #607 anyway

@sashahilton00
Copy link
Member

Well I guess it's the weekend of releases. Am pushing out 0.1.6 which fixes this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants