Support ALPN only if the underlying macOS supports ALPN methods

[kzys]

No description provided.

[kzys]

Not so sure why 10.13 is failing. Maybe http://www.openradar.me/34790589 ?

I probably need to skip the ALPN-related tests when the symbol is missing, but I'd like to discuss about the direction of this change first.

[kornelski]

Wouldn't it be better to use weak linking instead of dlopen?

[kzys]

Sadly weak linkage is nightly only, iirc.
rust-lang/rust#29603

Another option is targeting newer versions of macOS by default, like https://github.com/servo/core-foundation-rs

To enable features added in macOS 10.8, set Cargo feature mac_os_10_8_features. To have both 10.8 features and 10.7 compatibility, also set mac_os_10_7_support. Setting both requires weak linkage, which is is a nighty-only feature as of Rust 1.19.

[kzys]

Hmm, "weak!" macro on libstd looks handy, compared to having lazy_static and dlopen.

https://github.com/rust-lang/rust/blob/b559710e58427213d6f82008206c22cf3d76b4c4/src/libstd/sys/unix/pipe.rs#L25

https://github.com/rust-lang/rust/blob/b559710e58427213d6f82008206c22cf3d76b4c4/src/libstd/sys/unix/weak.rs

[kornelski]

Is ALPN available only for macOS 10.13+? If so, that's a bit high to aim for. I think currently 10.8-10.9 is the reasonable baseline.

So maybe ALPN itself could be a Cargo feature? It could be auto enabled when 10.13+ is set as minimum, and opt-in (needing weak/nightly) otherwise.

[sfackler]

If we're going to move to a dlopen-based approach for functionality added in newer OS versions, I think we'll want to do it for the entire library rather than as a one-off for ALPN.

[sfackler]

@kornelski ALPN support is currently only enabled if you turn on the OSX_10_13 Cargo feature - this PR is trying to change that to be detected at runtime.

[kornelski]

I'm suggesting

[features]
alpn = []
OSX_10_13 = ["OSX_10_12", "alpn"]

and if both OSX_10_13 and alpn are set, use it unconditionally. If alpn is set, but not OSX_10_13, then use it via weak linking.

@kzys righly noted such approach is used by core foundation already: https://github.com/servo/core-foundation-rs/search?q=weak&unscoped_q=weak

[kzys]

Works me. I've updated this pull request. Both "cargo test --features OSX_10_13" and "cargo test --features alpn" are working on my MacBook (macOS 10.13.6), while I haven't updated .circleci/config.yml yet.

[kzys]

@sfackler Could you take a look?

[kornelski]

Thanks for the PR, @kzys!