Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign up[msvc] Convert from make to cmake for native Windows builds #63
Conversation
| let target = env::var("TARGET").unwrap(); | ||
| if target.contains("windows") { | ||
| let dst = cmake::Config::new("harfbuzz").build(); | ||
| println!("cargo:rustc-link-search=native={}", dst.display()); |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
vvuk
Jun 24, 2016
Author
Contributor
Because of the install(TARGETS harfbuzz ARCHIVE DESTINATION .). I can make this go to lib for consistency if you'd prefer.
This comment has been minimized.
This comment has been minimized.
metajack
Jul 11, 2016
Contributor
I'd prefer it be consistent, but if you feel strongly otherwise that's fine.
| println!("cargo:rustc-link-search=native={}", dst.display()); | ||
| println!("cargo:rustc-link-lib=static=harfbuzz"); | ||
| } else { | ||
| assert!(Command::new("make") |
This comment has been minimized.
This comment has been minimized.
metajack
Jun 23, 2016
Contributor
Now we have two makefiles to maintain. Can we not get this down to one? I suppose we could do this in a followup.
This comment has been minimized.
This comment has been minimized.
vvuk
Jun 24, 2016
Author
Contributor
Yep, we should do this as a followup. Trying to convert this to a cross-platform cmakelist was actually painful, since Harfbuzz's configure generates a config.h with a number of important bits in it that I didn't want to track down right then. Certainly possible to do, but for an infrequently-changing library like this I didn't think it was worth it right away.
This comment has been minimized.
This comment has been minimized.
|
r=me with or without the lib dir thing. Up to you. |
|
@bors-servo delegate+ |
|
|
Confusion happened in servo/rust-harfbuzz#63 because Homu failed to explain what "approval" meant in the current context.
vvuk commentedJun 22, 2016
•
edited by larsbergstrom
This change is