Make cross-compilation possible #40
Conversation
This is achieved by using specific env_vars for the target. This allows the crate being compiled for diesel_migrations for the host. While also compiling it with the correct lib dir for the target.
build.rs
Outdated
| let path = PathBuf::from(&pg_lib_path); | ||
| if !path.exists() { | ||
| panic!("Folder {:?} doesn't exist in the configured path: {:?}", pq_lib_dir_for_target, path); | ||
| } | ||
| println!("{:?} = {:?}", pq_lib_dir_for_target, pg_lib_path); // list in output for small debuggability |
There was a problem hiding this comment.
This check is not performed for the "normal" PQ_LIB_DIR variable at all. Either we need to change that there or remove it from here.
There was a problem hiding this comment.
Added to the "normal" variable as well. As I find it to be good practice to give as many hints as possible as to why something fails.
build.rs
Outdated
| println!("cargo:rerun-if-env-changed=PQ_LIB_DIR"); | ||
| println!("PQ_LIB_DIR = {:?}", env::var("PQ_LIB_DIR")); | ||
| env::var("PQ_LIB_DIR") |
There was a problem hiding this comment.
I would like to unify this block with that one below as they are identical.
There was a problem hiding this comment.
I refactored it into a function
It doesn't maintain the static linkage fixes from #28 - there are still linking errors without these two lines: https://github.com/sgrif/pq-sys/pull/28/files#diff-d0d98998092552a1d3259338c2c71e118a5b8343dd4703c0c7f552ada7f9cb42R111-R112 (which is probably out of scope for this PR, but just adding a note so #28 isn't closed). |
README.md
Outdated
| * First, if the environment variable `PQ_LIB_DIR` is set, it will use its value | ||
| * Second it will look for an environment variable in the format of `PQ_LIB_DIR_{TARGET}` | ||
| where `{TARGET}` gets replaced by the Target environment variable set for cross-compilation |
There was a problem hiding this comment.
These should be switched, as the current implementation prefers PQ_LIB_DIR_{TARGET} over PQ_LIB_DIR (which is fine).
build.rs
Outdated
| @@ -127,6 +146,19 @@ fn configured_by_vcpkg() -> bool { | |||
| false | |||
| } | |||
|
|
|||
| fn use_general_lib_dir() -> Result<String, VarError>{ | |||
There was a problem hiding this comment.
I would change this function in such a way that it accepts the name of a environment variable as input. This would enable reusing the same function for target specific variables as well and unify even more code there.
build.rs
Outdated
| println!("cargo:rerun-if-env-changed={}", pq_lib_dir_for_target); | ||
| if let Ok(pg_lib_path) = env::var(pq_lib_dir_for_target.clone()) { | ||
| let path = PathBuf::from(&pg_lib_path); | ||
| if !path.exists() { | ||
| panic!("Folder {:?} doesn't exist in the configured path: {:?}", pq_lib_dir_for_target, path); | ||
| } | ||
| println!("{:?} = {:?}", pq_lib_dir_for_target, pg_lib_path); // list in output for small debuggability | ||
| Ok(pg_lib_path) | ||
| } | ||
| else{ | ||
| use_general_lib_dir() | ||
| } |
There was a problem hiding this comment.
With the generalized version of use_general_lib_dir() this could be written as
| println!("cargo:rerun-if-env-changed={}", pq_lib_dir_for_target); | |
| if let Ok(pg_lib_path) = env::var(pq_lib_dir_for_target.clone()) { | |
| let path = PathBuf::from(&pg_lib_path); | |
| if !path.exists() { | |
| panic!("Folder {:?} doesn't exist in the configured path: {:?}", pq_lib_dir_for_target, path); | |
| } | |
| println!("{:?} = {:?}", pq_lib_dir_for_target, pg_lib_path); // list in output for small debuggability | |
| Ok(pg_lib_path) | |
| } | |
| else{ | |
| use_general_lib_dir() | |
| } | |
| use_general_lib_dir(pq_lib_dir_for_target).or_else(|_| use_general_lib_dir("PQ_LIB_DIR")) |
There was a problem hiding this comment.
I still have to get used to more functional programming and not be as imperativ as I used to. Thanks for showing me that 👍
|
Thanks for the update. Looks good now 👍 |
This is achieved by using specific env_vars for the target.
This allows the crate being compiled for diesel_migrations for the host.
While also compiling it with the correct lib dir for the target.
I use this version (including the fixes of #28) of the build.rs to make cross compilation from a M1 Mac to x86_64 and it works great. 👍