Closed Bug 1369665 Opened 2 years ago Closed 2 years ago

"mach vendor rust" command has to export the OpenSSL lib folder

Categories

(Firefox Build System :: General, enhancement)

enhancement
Not set

Tracking

(firefox55 fixed)

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: whimboo, Assigned: whimboo)

Details

Attachments

(1 file)

Right now linking of cargo-vendor will fail because we do not export the openssl lib folder, but only include, and that twice:

https://dxr.mozilla.org/mozilla-central/rev/bdb2387396b4a74dfefb7c983733eed3625e906a/python/mozbuild/mozbuild/vendor_rust.py#86-93

What's missing here is something like:

OPENSSL_LIB_DIR=`brew --prefix openssl`/lib
Assignee: nobody → hskupin
Status: NEW → ASSIGNED
So for Servo they took a similar change:
https://github.com/servo/servo/pull/13225
Comment on attachment 8873799 [details]
Bug 1369665 - "mach vendor rust" has to export the OpenSSL lib folder.

https://reviewboard.mozilla.org/r/145228/#review149282

Thanks for the patch! This is definitely and improvement, so I'll go ahead and merge. What system did you get the link failure on? On mine (macOS 10.12.5 + homebrew) `cargo install cargo-vendor` works fine, and ends up linked to the correct libssl.dylib. It looks like there's code in recent releases of openssl-sys to find the homebrew openssl install, so I'm wondering if we can just remove these variables entirely.

::: commit-message-aeb3d:6
(Diff revision 1)
> +Bug 1369665 - "mach vendor rust" has to export the OpenSSL lib folder.
> +
> +It's not enough to only export the include folder for openssl, but for
> +linking the lib folder is also necessary. Also DEP_OPENSSL_INCLUDE
> +shouldn't be set directly, because it's set by Cargo itself based
> +on the output of openssl-sys's build script.

Can you explain how this happens? The [docs](http://doc.crates.io/build-script.html#the-links-manifest-key) suggest the `DEP_FOO` variables are explicitly set by build.rs, as a way to communicate ad-hoc metadata.

I didn't find any code in cargo for generating something like this for C-language include paths.

The point may be moot now though. I don't remember why I added this line, but nothing in the current openssl crate uses it.
Attachment #8873799 - Flags: review?(giles) → review+
Pushed by rgiles@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d62f2d63e674
"mach vendor rust" has to export the OpenSSL lib folder. r=rillian
https://hg.mozilla.org/mozilla-central/rev/d62f2d63e674
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Comment on attachment 8873799 [details]
Bug 1369665 - "mach vendor rust" has to export the OpenSSL lib folder.

https://reviewboard.mozilla.org/r/145228/#review149282

I also have MacOS 10.12.5 and Homebrew 1.2.1 installed. Here the output from `brew list openssl`:

% brew list openssl
/usr/local/Cellar/openssl/1.0.2k/bin/c_rehash
/usr/local/Cellar/openssl/1.0.2k/bin/openssl
/usr/local/Cellar/openssl/1.0.2k/include/openssl/ (75 files)
/usr/local/Cellar/openssl/1.0.2k/lib/libcrypto.1.0.0.dylib
/usr/local/Cellar/openssl/1.0.2k/lib/libssl.1.0.0.dylib
/usr/local/Cellar/openssl/1.0.2k/lib/engines/ (12 files)
/usr/local/Cellar/openssl/1.0.2k/lib/pkgconfig/ (3 files)
/usr/local/Cellar/openssl/1.0.2k/lib/ (4 other files)
/usr/local/Cellar/openssl/1.0.2k/share/man/ (1592 files)

So not sure which version you have, and if you have those env variables already globally setup?

> Can you explain how this happens? The [docs](http://doc.crates.io/build-script.html#the-links-manifest-key) suggest the `DEP_FOO` variables are explicitly set by build.rs, as a way to communicate ad-hoc metadata.
> 
> I didn't find any code in cargo for generating something like this for C-language include paths.
> 
> The point may be moot now though. I don't remember why I added this line, but nothing in the current openssl crate uses it.

Sorry, I just copied what several people mentioned across different github related issues for Cargo. And given that the DEP variable didn't help, I trusted that.
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.