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.

Sign up for GitHub

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

Cross compile fixes #153

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Cross compile fixes #153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
71f3518
Remove compilation of all recursive C files in config dir
madwizard-thomas Nov 13, 2023
aa88305
Refactor build script: separate library and indings in separate funct…
madwizard-thomas Nov 10, 2023
4f3fdd8
Add library and raw-bindings features to lvgl-sys.
madwizard-thomas Nov 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lvgl-sys/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ cc = "1.0.79"
bindgen = "0.65.1"

[features]
library = []
raw-bindings = []
use-vendored-config = []
drivers = []
rust_timer = []
226 changes: 127 additions & 99 deletions lvgl-sys/build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#[cfg(feature = "library")]
use cc::Build;
#[cfg(feature = "drivers")]
use std::collections::HashSet;
Expand Down Expand Up @@ -27,25 +28,55 @@ fn main() {
let project_dir = canonicalize(PathBuf::from(env::var("CARGO_MANIFEST_DIR").unwrap()));
let shims_dir = project_dir.join("shims");
let vendor = project_dir.join("vendor");
let lvgl_src = vendor.join("lvgl").join("src");
#[cfg(feature = "rust_timer")]
let timer_shim = vendor.join("include").join("timer");
println!("cargo:rerun-if-env-changed={}", CONFIG_NAME);
let lv_config_dir = get_conf_path(&vendor);
let font_extra_src: Option<PathBuf> = get_font_extra_dir();
if let Some(p) = &font_extra_src {
println!("cargo:rerun-if-changed={}", p.to_str().unwrap())
}

let conf = BuildConf {
lv_config_dir: lv_config_dir.as_path(),
vendor: vendor.as_path(),
shims_dir: &shims_dir,
font_extra_src: font_extra_src.as_ref().map(PathBuf::as_path),
};

#[cfg(feature = "library")]
compile_library(&conf);

let font_extra_src: Option<PathBuf>;
generate_bindings(&conf);
}

fn get_font_extra_dir() -> Option<PathBuf> {
if let Ok(v) = env::var("PWD") {
let current_dir = canonicalize(PathBuf::from(v));
font_extra_src = {
if let Ok(p) = env::var("LVGL_FONTS_DIR") {
Some(canonicalize(PathBuf::from(p)))
} else if current_dir.join("fonts").exists() {
Some(current_dir.join("fonts"))
} else {
None
}
};
if let Ok(p) = env::var("LVGL_FONTS_DIR") {
Some(canonicalize(PathBuf::from(p)))
} else if current_dir.join("fonts").exists() {
Some(current_dir.join("fonts"))
} else {
None
}
} else {
font_extra_src = None
None
}
}

struct BuildConf<'a> {
lv_config_dir: &'a Path,
vendor: &'a Path,
shims_dir: &'a Path,
font_extra_src: Option<&'a Path>,
}

#[cfg(feature = "library")]
fn compile_library(conf: &BuildConf) {
let vendor = conf.vendor;

let lvgl_src = vendor.join("lvgl").join("src");
#[cfg(feature = "rust_timer")]
let timer_shim = vendor.join("include").join("timer");

// Some basic defaults; SDL2 is the only driver enabled in the provided
// driver config by default
Expand All @@ -58,93 +89,27 @@ fn main() {
#[cfg(feature = "drivers")]
let drivers = vendor.join("lv_drivers");

let lv_config_dir = {
let conf_path = env::var(CONFIG_NAME)
.map(PathBuf::from)
.unwrap_or_else(|_| {
match std::env::var("DOCS_RS") {
Ok(_) => {
// We've detected that we are building for docs.rs
// so let's use the vendored `lv_conf.h` file.
vendor.join("include")
}
Err(_) => {
#[cfg(not(feature = "use-vendored-config"))]
panic!(
"The environment variable {} is required to be defined",
CONFIG_NAME
);

#[cfg(feature = "use-vendored-config")]
vendor.join("include")
}
}
});

if !conf_path.exists() {
panic!(
"Directory {} referenced by {} needs to exist",
conf_path.to_string_lossy(),
CONFIG_NAME
);
}
if !conf_path.is_dir() {
panic!("{} needs to be a directory", CONFIG_NAME);
}
if !conf_path.join("lv_conf.h").exists() {
panic!(
"Directory {} referenced by {} needs to contain a file called lv_conf.h",
conf_path.to_string_lossy(),
CONFIG_NAME
);
}
#[cfg(feature = "drivers")]
if !conf_path.join("lv_drv_conf.h").exists() {
panic!(
"Directory {} referenced by {} needs to contain a file called lv_drv_conf.h",
conf_path.to_string_lossy(),
CONFIG_NAME
);
}

if let Some(p) = &font_extra_src {
println!("cargo:rerun-if-changed={}", p.to_str().unwrap())
}

println!(
"cargo:rerun-if-changed={}",
conf_path.join("lv_conf.h").to_str().unwrap()
);
#[cfg(feature = "drivers")]
println!(
"cargo:rerun-if-changed={}",
conf_path.join("lv_drv_conf.h").to_str().unwrap()
);
conf_path
};

#[cfg(feature = "drivers")]
{
println!("cargo:rerun-if-env-changed=LVGL_INCLUDE");
println!("cargo:rerun-if-env-changed=LVGL_LINK");
}

let mut cfg = Build::new();
if let Some(p) = &font_extra_src {
if let Some(p) = conf.font_extra_src {
add_c_files(&mut cfg, p)
}
add_c_files(&mut cfg, &lvgl_src);
add_c_files(&mut cfg, &lv_config_dir);
add_c_files(&mut cfg, &shims_dir);
add_c_files(&mut cfg, conf.shims_dir);
#[cfg(feature = "drivers")]
add_c_files(&mut cfg, &drivers);

cfg.define("LV_CONF_INCLUDE_SIMPLE", Some("1"))
.include(&lvgl_src)
.include(&vendor)
.warnings(false)
.include(&lv_config_dir);
if let Some(p) = &font_extra_src {
.include(conf.lv_config_dir);
if let Some(p) = conf.font_extra_src {
cfg.includes(p);
}
#[cfg(feature = "rust_timer")]
Expand All @@ -156,17 +121,27 @@ fn main() {

cfg.compile("lvgl");

#[cfg(feature = "drivers")]
link_extra.split(',').for_each(|a| {
println!("cargo:rustc-link-lib={a}");
//println!("cargo:rustc-link-search=")
});
}
fn generate_bindings(conf: &BuildConf) {
let mut cc_args = vec![
"-DLV_CONF_INCLUDE_SIMPLE=1",
"-I",
lv_config_dir.to_str().unwrap(),
conf.lv_config_dir.to_str().unwrap(),
"-I",
vendor.to_str().unwrap(),
conf.vendor.to_str().unwrap(),
"-fvisibility=default",
];

// Set correct target triple for bindgen when cross-compiling
let target = env::var("TARGET").expect("Cargo build scripts always have TARGET");
let target = env::var("CROSS_COMPILE").map_or_else(
|_| env::var("TARGET").expect("Cargo build scripts always have TARGET"),
|c| c.trim_end_matches('-').to_owned(),
Copy link
Collaborator

Choose a reason for hiding this comment

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

This may be set incorrectly or by some other process (or accidentally left in a shell); may be worth falling back to the TARGET var if compile fails with this and warn

);
let host = env::var("HOST").expect("Cargo build scripts always have HOST");
if target != host {
cc_args.push("-target");
Expand Down Expand Up @@ -214,11 +189,11 @@ fn main() {

let out_path = PathBuf::from(env::var("OUT_DIR").unwrap());
let bindings =
bindgen::Builder::default().header(shims_dir.join("lvgl_sys.h").to_str().unwrap());
let bindings = add_font_headers(bindings, &font_extra_src);
bindgen::Builder::default().header(conf.shims_dir.join("lvgl_sys.h").to_str().unwrap());
let bindings = add_font_headers(bindings, conf.font_extra_src);
#[cfg(feature = "drivers")]
let bindings = bindings
.header(shims_dir.join("lvgl_drv.h").to_str().unwrap())
.header(conf.shims_dir.join("lvgl_drv.h").to_str().unwrap())
.parse_callbacks(Box::new(ignored_macros));
//#[cfg(feature = "rust_timer")]
//let bindings = bindings.header(shims_dir.join("rs_timer.h").to_str().unwrap());
Expand All @@ -236,21 +211,73 @@ fn main() {
bindings
.write_to_file(out_path.join("bindings.rs"))
.expect("Can't write bindings!");
}

fn get_conf_path(vendor: &PathBuf) -> PathBuf {
let conf_path = env::var(CONFIG_NAME)
.map(PathBuf::from)
.unwrap_or_else(|_| {
match std::env::var("DOCS_RS") {
Ok(_) => {
// We've detected that we are building for docs.rs
// so let's use the vendored `lv_conf.h` file.
vendor.join("include")
}
Err(_) => {
#[cfg(not(feature = "use-vendored-config"))]
panic!(
"The environment variable {} is required to be defined",
CONFIG_NAME
);

#[cfg(feature = "use-vendored-config")]
vendor.join("include")
}
}
});

if !conf_path.exists() {
panic!(
"Directory {} referenced by {} needs to exist",
conf_path.to_string_lossy(),
CONFIG_NAME
);
}
if !conf_path.is_dir() {
panic!("{} needs to be a directory", CONFIG_NAME);
}
if !conf_path.join("lv_conf.h").exists() {
panic!(
"Directory {} referenced by {} needs to contain a file called lv_conf.h",
conf_path.to_string_lossy(),
CONFIG_NAME
);
}
#[cfg(feature = "drivers")]
link_extra.split(',').for_each(|a| {
println!("cargo:rustc-link-lib={a}");
//println!("cargo:rustc-link-search=")
})
if !conf_path.join("lv_drv_conf.h").exists() {
panic!(
"Directory {} referenced by {} needs to contain a file called lv_drv_conf.h",
conf_path.to_string_lossy(),
CONFIG_NAME
);
}

println!(
"cargo:rerun-if-changed={}",
conf_path.join("lv_conf.h").to_str().unwrap()
);
#[cfg(feature = "drivers")]
println!(
"cargo:rerun-if-changed={}",
conf_path.join("lv_drv_conf.h").to_str().unwrap()
);
conf_path
}

fn add_font_headers(
bindings: bindgen::Builder,
dir: &Option<impl AsRef<Path>>,
) -> bindgen::Builder {
fn add_font_headers(bindings: bindgen::Builder, dir: Option<&Path>) -> bindgen::Builder {
if let Some(p) = dir {
let mut temp = bindings;
for e in p.as_ref().read_dir().unwrap() {
for e in p.read_dir().unwrap() {
let e = e.unwrap();
let path = e.path();
if !e.file_type().unwrap().is_dir()
Expand All @@ -265,6 +292,7 @@ fn add_font_headers(
}
}

#[cfg(feature = "library")]
fn add_c_files(build: &mut cc::Build, path: impl AsRef<Path>) {
for e in path.as_ref().read_dir().unwrap() {
let e = e.unwrap();
Expand Down
2 changes: 2 additions & 0 deletions lvgl-sys/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@

include!(concat!(env!("OUT_DIR"), "/bindings.rs"));

#[cfg(feature = "raw-bindings")]
pub fn _bindgen_raw_src() -> &'static str {
include_str!(concat!(env!("OUT_DIR"), "/bindings.rs"))
}

#[cfg(feature = "library")]
mod string_impl;

#[cfg(test)]
Expand Down
4 changes: 2 additions & 2 deletions lvgl/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ keywords = ["littlevgl", "lvgl", "graphical_interfaces"]
build = "build.rs"

[dependencies]
lvgl-sys = { version = "0.6.2", path = "../lvgl-sys" }
lvgl-sys = { version = "0.6.2", path = "../lvgl-sys", features = ["library"]}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Tiny nit; here (and line 80) add a space before the curly brace for stylistic consistency

cty = "0.2.2"
embedded-graphics = { version = "0.8.0", optional = true }
cstr_core = { version = "0.2.6", default-features = false, features = ["alloc"] }
Expand Down Expand Up @@ -77,7 +77,7 @@ unsafe_no_autoinit = []
quote = "1.0.23"
proc-macro2 = "1.0.51"
lvgl-codegen = { version = "0.6.2", path = "../lvgl-codegen" }
lvgl-sys = { version = "0.6.2", path = "../lvgl-sys" }
lvgl-sys = { version = "0.6.2", path = "../lvgl-sys", features = ["raw-bindings"]}
Copy link
Collaborator

Choose a reason for hiding this comment

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

v2 of the cargo feature resolver does not unify features on crates compiled multiple times. If this and the above both specify both features for lvgl-sys (instead of just 1 each), it should unify and so the double-warnings might disappear?

Copy link
Author

@madwizard-thomas madwizard-thomas Nov 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

That defeats the entire purpose of this PR, which is to prevent building the library for lvgl's build.rs executable
The library will always be built twice, since one is compiled for xtensa (or riscv), the other for x86_64. So there is no way to unify that.

Copy link
Author

Choose a reason for hiding this comment

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

Although this does only apply to cross-compilation, if target == host (no cross compile) it may compile it twice unnecessarily as well.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not check for that in the build script & enable features from there by printlning cargo:rustc-cfg=whatever? Beyond TARGET, cargo also exports HOST, so a simple check together with the previous one might work (on this, maybe just check if those are the same instead? I wasn't able to find any docs on a CROSS_COMPILE variable)

Copy link
Author

Choose a reason for hiding this comment

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

When lvgl-sys is compiled for the build script executable, target is always the same as host (since build scripts must run on the host). So in this case there is no way to find out the actual target. CROSS_COMPILE is just some standard used, the cc crate supports it so I thought it made sense to use it for the bindings.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we gate this behind a flag or feature, then, so that we don't always compile twice? Feels like a pretty big cost still.

Copy link

Choose a reason for hiding this comment

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

Is this the only thing preventing this PR from landing?

If so, this can just have a no_cross_compile feature that enables both library and raw-bindings features of the lvgl-sys crate.

And this will apply to the normal dependency and the build dependency confirmed


[dev-dependencies]
embedded-graphics-simulator = "0.5.0"
Expand Down
Loading