Skip to content

Commit

Permalink
Assume tiff to avoid unnecessary libproj builds.
Browse files Browse the repository at this point in the history
This removes the bundled_proj_tiff feature (added in
https://github.com/georust/proj/pull/58/files), and we now assume all
proj installations support tiff. Meaning the user either:
1. If the user has a recent system installation of libproj, it supports
   libtiff.
2. Else if the user has no recent system installation of libproj,
   build.rs will compile it from source, which requires linking in
   libtiff

The previous "bundled_proj_tiff" unfortunately conflated needing tiff
with needing to build from source, even though most system proj installs
will already support tiff.

proj-sys requires a compatible libproj installation. build.rs will
either use a pre-existing system installation of libproj or build
libproj from source. Building from source takes a while, so it's
preferable to use the system installation if it's compatible.

tiff support is used by libproj's network grid.  tiff support is
required by libproj by default, though it's possible to opt out.

When Apple first released aarch64 (M1), libtiff was failing to build,
so as a stop gap we added a "bundled_proj_tiff" feature which:
1. forced a from-source build
2. explicitly enabled tiff support, else tiff support would be disabled,
   allowing us to build the proj crate on aarch64

The underlying build failures in libtiff have now been fixed, so the
original motivation for this feature no longer exists.

There was a cost associated with keeping it - unnecessarily triggering
source builds from, e.g. georust/geo crate, whose `proj/network` feature
wants to ensure tiff is enabled. Given that most installations will have
this feature, I think it will give most of our users a better experience
if we can avoid the compile.

To be clear, there is potential downside with this approach. It's still
conceivable that environments exists where tiff is not, or can not be
installed, but weighing that against the more common case of having
libproj installed with the default configuration, and I think this
approach wins.

If this poops on too many parties, we can revisit something like the
former behavior in a way that's less deleterious to the default use
case - e.g. have a `tiff` feature that will still use the local install
if it supports tiff.
  • Loading branch information
michaelkirk committed Oct 20, 2021
1 parent 801f653 commit 9aca22c
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 12 deletions.
1 change: 0 additions & 1 deletion .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ jobs:
- "--features \"network bundled_proj\""
- "--features \"network geo-types\""
- "--features \"bundled_proj geo-types\""
- "--features \"bundled_proj bundled_proj_tiff \""
- "--features \"network bundled_proj geo-types\""
container:
image: ${{ matrix.container_image }}
Expand Down
3 changes: 1 addition & 2 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,8 @@ members = ["proj-sys"]
[features]
default = ["geo-types"]
bundled_proj = [ "proj-sys/bundled_proj" ]
bundled_proj_tiff = [ "proj-sys/bundled_proj_tiff" ]
pkg_config = [ "proj-sys/pkg_config" ]
network = ["bundled_proj_tiff", "reqwest"]
network = ["reqwest"]

[dev-dependencies]
approx = "0.3"
Expand Down
8 changes: 8 additions & 0 deletions proj-sys/CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
# UNRELEASED

- BREAKING: Remove `bundled_proj_tiff` feature and assume system libproj has
the default enabled tiff support. Otherwise, the current setup would
unnecessarily build libproj from source in some cases, e.g. the geo crate's
proj network integration would compile libproj from source.
- <https://github.com/georust/proj/pull/92>

# 0.20.1
- Fix docs to refer to correct libproj version

Expand Down
1 change: 0 additions & 1 deletion proj-sys/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ nobuild = []
bundled_proj = []
# `pkg_config` feature is deprecated and does nothing
pkg_config = []
bundled_proj_tiff = ["bundled_proj"]

[package.metadata.docs.rs]
features = [ "nobuild" ] # This feature will be enabled during the docs.rs build
12 changes: 4 additions & 8 deletions proj-sys/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,17 +95,15 @@ fn build_from_source() -> Result<std::path::PathBuf, Box<dyn std::error::Error>>
config.define("BUILD_PROJINFO", "OFF");
config.define("BUILD_PROJSYNC", "OFF");
config.define("ENABLE_CURL", "OFF");
let tiff_support = cfg!(feature = "bundled_proj_tiff");
config.define("ENABLE_TIFF", if tiff_support { "ON" } else { "OFF" });
let proj = config.build();
// Tell cargo to tell rustc to link libproj, and where to find it
// libproj will be built in $OUT_DIR/lib

//proj likes to create proj_d when configured as debug and on MSVC, so link to that one if it exists
if proj.join("lib").join("proj_d.lib").exists() {
println!("cargo:rustc-link-lib=static=proj_d");
println!("cargo:rustc-link-lib=static=proj_d");
} else {
println!("cargo:rustc-link-lib=static=proj");
println!("cargo:rustc-link-lib=static=proj");
}
println!(
"cargo:rustc-link-search=native={}",
Expand All @@ -121,9 +119,7 @@ fn build_from_source() -> Result<std::path::PathBuf, Box<dyn std::error::Error>>
);
// The PROJ library needs SQLite and the C++ standard library.
println!("cargo:rustc-link-lib=dylib=sqlite3");
if tiff_support {
println!("cargo:rustc-link-lib=dylib=tiff");
}
println!("cargo:rustc-link-lib=dylib=tiff");
if cfg!(target_os = "linux") {
println!("cargo:rustc-link-lib=dylib=stdc++");
} else if cfg!(target_os = "macos") {
Expand Down

0 comments on commit 9aca22c

Please sign in to comment.