Cargo features must be additive

[rmanoka]

The README talks about two feature flags that are mutually exclusive. However, as I understand, cargo features are additive. For instance, if two dependencies of a crate depend on this crate and enable the exlusive features, cargo would enable both while compiling this crate.

@michaelkirk linked a few good examples of how other crates handle these cases here. This thread in urlo also suggests going for a environment variable based approach.

I think the env. variable approach makes sense for our use-case, because the end user who is linking the final binary should really have the say, and not the intermediate dependencies that depend on proj.

[urschrei]

Ugh, that hadn't occurred to me. I have to say, I'm not a fan of the env var approach, because it depends on specifying settings outside Cargo.

[michaelkirk]

I've spent some time trying to understand "the right way" to do sys crates, and my main take away is that it's more art than science.

I agree that features should, at least in general, be additive.

And yet, there are some examples of forcing a src build via features, even in popular crates, e.g. open-ssl's: vendored feature.

And this guide, which I've considered a mostly good rundown of best practices alludes to it as an option: https://kornel.ski/rust-sys-crate

Personally, the env approach makes more sense to me.

If we do keep the features, we should consider adding some kind of assert to enforce a compile time error if mutually exclusive options are set:
https://docs.rs/static_assertions/1.1.0/static_assertions/macro.assert_cfg.html

I think currently the situation is that the user must specify when they want to build from src. It'd be nice of the build script to detect whether or not proj is available, and automatically install from source if it's not. Exposing features/configs/env/whatever then would only be relevant in the rare circumstances when the user needs to override that behavior.

[urschrei]

I think currently the situation is that the user must specify when they want to build from src. It'd be nice of the build script to detect whether or not proj is available, and automatically install from source if it's not.

This would be great, and we should explore that. However, the pkg_config option is somewhat in tension with this, since it is in itself a fallback, and only exists because of complaints that the initial build.rs couldn't find pre-installed libproj.

So we could have a situation where we see whether we can find libproj "as-is". If not, we may have to check whether pkg-config is installed (I don't know whether this is feasible?), and try to use that if it is, finally trying to install from bundled source.

[michaelkirk]

Ah, interesting. Is there a reason that we don't just depend on pkg-config by default?

It seems pretty common practice:

https://github.com/sfackler/rust-openssl/blob/master/openssl-sys/Cargo.toml#L23
https://github.com/rust-lang/libz-sys/blob/main/Cargo.toml#L28
https://github.com/rust-lang/git2-rs/blob/master/libgit2-sys/Cargo.toml#L25

[urschrei]

Ahh. So we could use the pkg-config crate and catch the relevant errors, moving on to source install if necessary.

[michaelkirk]

Ah, I hadn't really looked into it. That makes sense though.

If that approach is amenable, I'll follow up with a PR to -sys later this week.

[michaelkirk]

I kind of hijacked the thread and don't know that we've addressed @rmanoka's original concern.

A more concrete proposal:

By default we should rely on pkg-config to find proj. We should fall back to a src build if pkg-config errors or fails to find an appropriate version of proj.

We should expose a way to override the default behavior and force building the bundled source. For this I have a slight preference of using ENV variables rather than features, but don't feel too strongly.

[urschrei]

I think if we subsume the pkg-config feature into the default build, we end up with only one optional feature (bundled_proj) anyway, so we don't need to worry too much about env vars, although it's not future-proof – my expectation is that the crate will remain stable with regard to new features, because proj changes very rarely (proj.4 was feature-stable for a decade iirc)

[michaelkirk]

That's a good point.

[rmanoka]

Using pkg-config and figuring what to link to sounds very cool! I'm okay with an optional feature flag for opting for building the in-crate source, although it is a bit hard for end user to config unless proj is their direct dependency (I suppose they can always explicitly add proj as a direct dep). Anyway, pkg-config seems to allow environment variables to disable its search, so that route should anyway be available.

@urschrei @michaelkirk Thank you for discussing this!