openssl-sys constantly recompiling since 0.9.100 with OPENSSL_LIB_DIR, OPENSSL_INCLUDE_DIR set

[alexheretic]

openssl-sys constantly recompiling since 0.9.100. This has significant impact on incremental builds.

Steps to reproduce

New project with

[dependencies]
openssl = "0.10"

Incremental build output:

$ cargo check
   Compiling openssl-sys v0.9.101
   Compiling openssl v0.10.64
    Checking openssl-recom v0.1.0 (/home/alex/tmp/openssl-recom)
    Finished dev [unoptimized + debuginfo] target(s) in 1.24s
$ cargo check
   Compiling openssl-sys v0.9.101
   Compiling openssl v0.10.64
    Checking openssl-recom v0.1.0 (/home/alex/tmp/openssl-recom)
    Finished dev [unoptimized + debuginfo] target(s) in 1.29s

Downgrade to older openssl-sys

$ cargo update -p openssl --precise 0.10.63
$ cargo update -p openssl-sys --precise 0.9.99

Incremental build output: (Does not recompile each time).

$ cargo check
    Finished dev [unoptimized + debuginfo] target(s) in 0.00s
$ cargo check
    Finished dev [unoptimized + debuginfo] target(s) in 0.00s

[alex]

I'm not able to reproduce this with the latest release. Can you run cargo with -v and share the output?

[alexheretic]

I had custom set OPENSSL_LIB_DIR, OPENSSL_INCLUDE_DIR env vars which after removing resolved the issue (and I don't seem to need them anymore).

It's quite possible then that this issue may not affect many / any other use scenarios. So I'll close, sorry for the noise.

[Skepfyr]

Ah so interestingly if I have OPENSSL_LIB_DIR and OPENSSL_INCLUDE_DIR set then it recompiles openssl-sys, every time. That seems bad.

[Skepfyr]

I think the problem is this line:

rust-openssl/openssl-sys/build/main.rs

Lines 86 to 88 in 3439059

	if let Some(printable_include) = include_dir.join("openssl").to_str() {
	println!("cargo:rerun-if-changed={}", printable_include);
	}

If I set OPENSSL_INCLUDE_DIR to /usr/include/openssl (rather than /usr/include) then the path /usr/include/openssl/openssl doesn't exist, causing cargo to rebuild every time.

[alex]

Interesting. CC @reaperhulk

…

On Thu, Feb 22, 2024, 9:01 AM Jack Rickard ***@***.***> wrote: I think the problem is this line: https://github.com/sfackler/rust-openssl/blob/3439059e1dbb0df5d0e29ac64670735c65a2aa6c/openssl-sys/build/main.rs#L86-L88 If I set OPENSSL_INCLUDE_DIR to /usr/include/openssl (rather than /usr/include) then the path /usr/include/openssl/openssl doesn't exist, causing cargo to rebuild every time. — Reply to this email directly, view it on GitHub <#2183 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAAGBGRJPUE7MD3QU35IRLYU5FVBAVCNFSM6AAAAABDUXENIOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNJZGUYTCNBQHE> . You are receiving this because you commented.Message ID: ***@***.***>

[Skepfyr]

Although thinking about it, I think I've just misconfigured it by specifying it as /usr/include/openssl, maybe we should just be a bit more explicit about checking that the include directory looks correct, we could throw an error if it doesn't have an openssl directory underneath it. (Although not sure if that would break anyone's current setup.

[reaperhulk]

I think erroring is highly likely to break somebody, but we could check if the path exists (fs::metadata(path).is_ok()) and only emit the rebuild instruction then.

I don't love the complexity that is arising here though. If we make this change we will emit this rerun event if it's not vendored and if it is able to find a $INCLUDE/openssl dir, but there's no user visible indicator of this logic chain.

While the previous behavior was not ideal, it also probably only affected a small subset of folks who regularly upgrade their OpenSSL and also have rust intermediate caching systems. It just so happens it directly affected the project @alex and I work on 😄

@alex @sfackler do you two have an opinion? I can add the existence check or I can revert the whole thing -- up to you!

[alex]

I think silently ignoring it if the directory doesn't exist there is the best path

[reaperhulk]

#2187