fix: android builds #58
Conversation
|
Can you run clippy as well? |
build.rs
Outdated
| } | ||
| } | ||
|
|
||
| fn host_tag() -> String { |
There was a problem hiding this comment.
you only use this to interpolate into another String later; I would make this return &'static str and remove the to_string, saves you an allocation that you aren't using anyway.
| } else if cfg!(target_os = "macos") { | ||
| "darwin-x86_64" | ||
| } else { | ||
| panic!("host os is not supported") |
There was a problem hiding this comment.
I don't know much about android so it's hard for me to know if this is right or wrong, but I do know that sometimes I am frustrated by proactive checks like this, when I'm on a system that's close enough to one of these where things will still work, but don't specifically because of the check. that's only something you can really know though, and maybe you just wouldn't want to support those cases anyway.
There was a problem hiding this comment.
Honestly not much can be done here as they only support macOS, Linux and Windows
build.rs
Outdated
| // There's a source.properties file in the ndk directory, which contains | ||
| let mut source_properties = | ||
| File::open(ndk_dir.join("source.properties")).expect("Couldn't open source.properties"); | ||
| let mut buf = "".to_string(); |
There was a problem hiding this comment.
this is a small style thing but I like String::new() here. does the exact same thing I just think it looks way cleaner.
|
|
||
| let compiler = build.get_compiler(); | ||
| // Note: it's possible to use Clang++ explicitly on Windows as well, so this check | ||
| // should be specifically for "is target compiler MSVC" and not "is target OS Windows". |
| /// Get NDK major version from source.properties | ||
| fn ndk_major_version(ndk_dir: &Path) -> u32 { | ||
| // Capture version from the line with Pkg.Revision | ||
| let re = Regex::new(r"Pkg.Revision = (\d+)\.(\d+)\.(\d+)").unwrap(); |
There was a problem hiding this comment.
this is just simple enough that I wonder if you need to bring in a full regex implementation here, that being said, it is easier, but if you did the parsing yourself, i bet compile times would be faster. hard to say, to be honest, but I thought I'd mention it.
No description provided.