Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade openssl, selectors, and cocoa #6740

Merged
merged 1 commit into from Jul 30, 2015
Merged

Conversation

@mbrubeck
Copy link
Contributor

mbrubeck commented Jul 24, 2015

This lets Servo use one version of bitflags for all dependencies.

r? @larsbergstrom or @Ms2ger

Review on Reviewable

@hoppipolla-critic-bot
Copy link

hoppipolla-critic-bot commented Jul 24, 2015

Critic review: https://critic.hoppipolla.co.uk/r/5645

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

@larsbergstrom
Copy link
Contributor

larsbergstrom commented Jul 24, 2015

@bors-servo
Copy link
Contributor

bors-servo commented Jul 24, 2015

📌 Commit 68b8060 has been approved by larsbergstrom

@bors-servo
Copy link
Contributor

bors-servo commented Jul 25, 2015

Testing commit 68b8060 with merge c9d4e72...

bors-servo pushed a commit that referenced this pull request Jul 25, 2015
Upgrade openssl, selectors, and cocoa

This lets Servo use one version of bitflags for all dependencies.

r? @larsbergstrom or @Ms2ger

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6740)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 25, 2015

💔 Test failed - gonk

@bors-servo
Copy link
Contributor

bors-servo commented Jul 25, 2015

The latest upstream changes (presumably #6616) made this pull request unmergeable. Please resolve the merge conflicts.

@jdm
Copy link
Member

jdm commented Jul 25, 2015

error: linking with `./fake-ld.sh` failed: exit code: 1
[linker command line elided for brevity]
note: /home/servo/.cargo/registry/src/github.com-0a35038f75765ae4/openssl-0.6.4/src/bio/mod.rs:63: error: undefined reference to 'BIO_set_mem_eof_return_shim'
/home/servo/.cargo/registry/src/github.com-0a35038f75765ae4/openssl-0.6.4/src/bio/mod.rs:75: error: undefined reference to 'BIO_eof_shim'
/home/servo/.cargo/registry/src/github.com-0a35038f75765ae4/openssl-0.6.4/src/ssl/mod.rs:452: error: undefined reference to 'SSL_CTX_set_read_ahead_shim'
/home/servo/.cargo/registry/src/github.com-0a35038f75765ae4/openssl-0.6.4/src/ssl/mod.rs:700: error: undefined reference to 'SSL_set_tlsext_host_name_shim'
collect2: error: ld returned 1 exit status

error: aborting due to previous error
Could not compile `b2s`.
@mbrubeck
Copy link
Contributor Author

mbrubeck commented Jul 25, 2015

It looks like the gonk build was broken by sfackler/rust-openssl@d0b769c. I'm not sure why the linker arguments passed to fake-ld.sh don't include the openssl-sys out directory or the openssl_shim library.

@mbrubeck
Copy link
Contributor Author

mbrubeck commented Jul 25, 2015

Possibly related to this override in ports/gonk/.cargo/config?

[target.arm-linux-androideabi.openssl]
rustc-flags = "-l crypto -l ssl"
@mbrubeck
Copy link
Contributor Author

mbrubeck commented Jul 27, 2015

More details on the Gonk problem:

The openssl-sys crate includes a small C library called openssl_shim which wraps some functions from OpenSSL. In older versions of openssl-sys, this shim was needed only for compatibility with old versions of OpenSSL. But in the latest version of openssl-sys, the shim is needed regardless of OpenSSL version.

The shim depends on preprocessor macros defined in the OpenSSL headers. The B2G "obj" directory we use in our Gonk build has compiled OpenSSL libraries, but it does not have the OpenSSL headers. This causes errors when building the shim.

We previously worked around the lack of headers by overriding the build script in ports/gonk/.cargo/config, which just skips building the shim. However, in the latest version, the shim is required. Skipping it causes the above errors at link time.

Possible solutions:

  • Get the necessary header files for the OpenSSL libraries in our B2G build dir, and enable the shim (remove the override from ports/gonk/.cargo/config).
  • Revert sfackler/rust-openssl@d0b769c and move the shimmed macros back into Rust code.
  • If we don't actually depend on any of the shimmed functions, we could hide them behind an optional feature in openssl-sys and disable this in Servo.
@metajack
Copy link
Contributor

metajack commented Jul 29, 2015

pinging @sfackler

Also, this was discussed at the meeting, but the result was not recorded here. I think that @Manishearth had suggested getting the headers in the right place. What will that take?

@larsbergstrom
Copy link
Contributor

larsbergstrom commented Jul 29, 2015

Oh, yeah, sorry - I was talking, so I think it got missed in the meeting minutes.

I lost my B2G build (I accidentally broke the linux VM image), but if we make a new one we should be able to add those files to the instructions:

tar zcf B2G.tgz B2G/bionic B2G/system B2G/frameworks/native/opengl/include B2G/external/zlib B2G/hardware/libhardware/include/hardware B2G/ndk/sources/cxx-stl B2G/prebuilts/gcc/linux-x86/arm/arm-linux-androideabi-4.7 B2G/out/target/product/flame/obj/ B2G/prebuilts/ndk/9/sources/cxx-stl/gnu-libstdc++/4.6/libs/armeabi/
larsberg@ubuntu:/mnt/more$ s3cmd put B2G.tgz s3://servo-rust/B2G/B2G.tgz
@sfackler
Copy link

sfackler commented Jul 29, 2015

There's actually a second shim to handle mapping the SSL_OP_* constants since their values change based on the openssl version: https://github.com/sfackler/rust-openssl/blob/master/openssl-sys/build.rs#L77.

If you're okay keeping precompiled binaries around, one option would be to add a OPENSSL_SHIM_PATH environment variable that'd allow you to point the buildscript to a prebuilt version of the shim.

@larsbergstrom
Copy link
Contributor

larsbergstrom commented Jul 30, 2015

I've added the openssl headers. They should be in B2G/external/openssl/include/openssl.

@mbrubeck mbrubeck force-pushed the mbrubeck:bitflags branch from 68b8060 to 384c492 Jul 30, 2015
@mbrubeck mbrubeck force-pushed the mbrubeck:bitflags branch from 384c492 to fd23df5 Jul 30, 2015
@mbrubeck mbrubeck force-pushed the mbrubeck:bitflags branch from fd23df5 to a13e237 Jul 30, 2015
@mbrubeck
Copy link
Contributor Author

mbrubeck commented Jul 30, 2015

@larsbergstrom
Copy link
Contributor

larsbergstrom commented Jul 30, 2015

@bors-servo
Copy link
Contributor

bors-servo commented Jul 30, 2015

📌 Commit a13e237 has been approved by larsbergstrom

@mbrubeck mbrubeck force-pushed the mbrubeck:bitflags branch from a13e237 to 63d39a4 Jul 30, 2015
@mbrubeck
Copy link
Contributor Author

mbrubeck commented Jul 30, 2015

@bors-servo r=larsbergstrom

(Forgot to "git add" a few more lines of Cargo.lock changes)

@bors-servo
Copy link
Contributor

bors-servo commented Jul 30, 2015

📌 Commit 63d39a4 has been approved by larsbergstrom

@bors-servo
Copy link
Contributor

bors-servo commented Jul 30, 2015

Testing commit 63d39a4 with merge df722ec...

bors-servo pushed a commit that referenced this pull request Jul 30, 2015
Upgrade openssl, selectors, and cocoa

This lets Servo use one version of bitflags for all dependencies.

r? @larsbergstrom or @Ms2ger

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6740)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 30, 2015

☀️ Test successful - android, gonk, linux1, linux2, linux3, mac1, mac2, mac3

@bors-servo bors-servo merged commit 63d39a4 into servo:master Jul 30, 2015
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@mbrubeck mbrubeck deleted the mbrubeck:bitflags branch May 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

8 participants
You can’t perform that action at this time.