Skip to content
This repository was archived by the owner on Jan 2, 2025. It is now read-only.

Cannot build tcod-rs on OS X 10.11 - as_ref(), as_bytes() errors #205

Closed
ysgard opened this issue Oct 15, 2015 · 11 comments · Fixed by #206
Closed

Cannot build tcod-rs on OS X 10.11 - as_ref(), as_bytes() errors #205

ysgard opened this issue Oct 15, 2015 · 11 comments · Fixed by #206

Comments

@ysgard
Copy link

ysgard commented Oct 15, 2015

Putting in place the workaround described in my earlier 'SDL include' issue (#204) yields a different error:

➜  tcod-rs git:(master) cargo build
   Compiling tcod-sys v3.0.0 (file:///Users/ysgard/Dropbox/Rust/tcod-rs)
   Compiling tcod v0.8.0 (file:///Users/ysgard/Dropbox/Rust/tcod-rs)
src/console.rs:479:39: 479:62 error: mismatched types:
 expected `&std::path::Path`,
    found `&core::convert::AsRef<std::path::Path>`
(expected struct `std::path::Path`,
    found trait core::convert::AsRef) [E0308]
src/console.rs:479                 Root::set_custom_font(self.font_path.as_ref(),
                                                         ^~~~~~~~~~~~~~~~~~~~~~~
src/console.rs:479:39: 479:62 help: run `rustc --explain E0308` to see a detailed explanation
src/console.rs:486:60: 486:70 error: no method named `as_bytes` found for type `&core::convert::AsRef<str>` in the current scope
src/console.rs:486             let c_title = CString::new(self.title.as_ref().as_bytes()).unwrap();
                                                                              ^~~~~~~~~~
error: aborting due to 2 previous errors
Could not compile `tcod`.

To learn more, run the command again with --verbose.

This is being built against the Rust nightly line:

➜  tcod-rs git:(master) rustc -V
rustc 1.5.0-nightly (81b3b27cf 2015-10-11)
@ysgard ysgard changed the title Cannot build tcod-rs on OS X 10.11 - Cannot build tcod-rs on OS X 10.11 - as_ref(), as_bytes() errors Oct 15, 2015
@zsparal
Copy link
Collaborator

zsparal commented Oct 15, 2015

Yep, saw this as well, it seems to be a breaking change in the nightly. I can't send a PR right away, but it can be solved by adding an explicit deref to the lines mentioned, example:

(*self.font_path).as_ref()
(*self.title).as_ref().as_bytes()

@ysgard
Copy link
Author

ysgard commented Oct 15, 2015

That fixes it for me as well. I'm still learning Rust - is this an accidental breakage, or a new direction the Rust team is leaning towards?

@zsparal
Copy link
Collaborator

zsparal commented Oct 15, 2015

I've no idea unfortunately, I had no time to keep up with changes in recent nightlies. It would be interesting to see, since they shouldn't introduce breaking changes before 2.0, and this change to deref semantics seems a bit unnecessary.

EDIT: thanks to Reddit, found the PR in question: rust-lang/rust#28811.

@zsparal
Copy link
Collaborator

zsparal commented Oct 15, 2015

Also @tomassedovic, these errors are both related to RootInitializer, specifically:

title: Box<AsRef<str> + 'a>
font_path: Box<AsRef<Path> + 'a>

Don't you think that if we're doing a heap allocation anyway (for the Box) then storing these members as String and Path respectively would be clearer? The RootInitializer is used once during the program's execution anyway, so even if it was slightly slower, the performance wouldn't really matter.

@tomassedovic
Copy link
Owner

Yeah, good point on just using the heap-allocated values anyway.

And I'm not too happy about this breaking change either. They're using crater, but tcod-rs and tcod-sys don't compile because of the missing SDL dependencies. So the Rust devs don't see any regressions from us.

I'd love us to compile cleanly there but I'm not sure how to go about doing that.

@zsparal
Copy link
Collaborator

zsparal commented Oct 16, 2015

I feel like that is more of a crater deficiency than a tcod-rs one. We cannot include every single dependency in the source tree... There should be a way to tell crater to fetch some popular Linux libraries (I cannot imagine we're the only ones having this problem).

As much as I don't like it, we could add the SDL sources (a'la libtcod), but SDL is popular enough that a build using the system version is likely beneficial.

@bstrie
Copy link

bstrie commented Oct 17, 2015

@brson, are you aware of this sort of hole in crater's coverage? Is there anything we can do to accommodate this?

@brson
Copy link

brson commented Oct 17, 2015

Yes, sadly only native deps that I've loaded into the crater docker image work.

@tomassedovic
Copy link
Owner

@brson is there a way to detect we're compiling under crater? This seems to boil down to the build script failing. But we only care about the Rust code in crater so if we could skip the native stuff, our code would be checked as well.

@brson
Copy link

brson commented Oct 26, 2015

Yes. If you look at one of the existing crater runs there are several env
vars defined. I'm on mobile now otherwise I'd just tell you one.
On Oct 22, 2015 5:44 AM, "Tomas Sedovic" [email protected] wrote:

@brson https://github.com/brson is there a way to detect we're compiling
under crater? This seems to boil down to the build script failing. But we
only care about the Rust code in crater so if we could skip the native
stuff, our code would be checked as well.


Reply to this email directly or view it on GitHub
#205 (comment).

@tomassedovic
Copy link
Owner

Thanks. I see CRATER_TASK_TYPE and a bunch of other ones (rust installer, cargo installer and crate file) there.

So we'll check for them in the build script and won't compile the non-rust dependencies in that case.

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

Successfully merging a pull request may close this issue.

5 participants