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 to rustc 1.3.0-dev (fddfd089b 2015-07-10) #6591

Merged
merged 3 commits into from Jul 15, 2015
Merged

Conversation

@SimonSapin
Copy link
Member

SimonSapin commented Jul 10, 2015

Depends on mozilla/webdriver-rust#12.

Fixes #6020.

Review on Reviewable

@hoppipolla-critic-bot
Copy link

hoppipolla-critic-bot commented Jul 10, 2015

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

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.

@SimonSapin SimonSapin force-pushed the rustup_2015-07-10 branch 2 times, most recently from d0ede43 to b2a87db Jul 10, 2015
@pcwalton
Copy link
Contributor

pcwalton commented Jul 10, 2015

Looks good to me, once upstream is merged feel free to land.

@SimonSapin
Copy link
Member Author

SimonSapin commented Jul 10, 2015

@bors-servo r=pcwalton

@bors-servo
Copy link
Contributor

bors-servo commented Jul 10, 2015

📌 Commit b2a87db has been approved by pcwalton

@bors-servo
Copy link
Contributor

bors-servo commented Jul 10, 2015

Testing commit b2a87db with merge 35ea678...

bors-servo pushed a commit that referenced this pull request Jul 10, 2015
Upgrade to rustc 1.3.0-dev (fddfd089b 2015-07-10)

Depends on mozilla/webdriver-rust#12.

Fixes #6020.

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

bors-servo commented Jul 10, 2015

💔 Test failed - android

@SimonSapin SimonSapin force-pushed the rustup_2015-07-10 branch from b2a87db to bbed054 Jul 10, 2015
@SimonSapin
Copy link
Member Author

SimonSapin commented Jul 10, 2015

@bors-servo r=pcwalton

(Re-ran ./mach update-cargo -p webdriver and ./mach update-cargo -p hyper now that mozilla/webdriver-rust#12 is merged.)

@bors-servo
Copy link
Contributor

bors-servo commented Jul 10, 2015

📌 Commit bbed054 has been approved by pcwalton

@bors-servo
Copy link
Contributor

bors-servo commented Jul 10, 2015

Testing commit bbed054 with merge a02e7d8...

bors-servo pushed a commit that referenced this pull request Jul 10, 2015
Upgrade to rustc 1.3.0-dev (fddfd089b 2015-07-10)

Depends on mozilla/webdriver-rust#12.

Fixes #6020.

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

bors-servo commented Jul 10, 2015

💔 Test failed - mac2

@SimonSapin
Copy link
Member Author

SimonSapin commented Jul 11, 2015

ld: warning: directory not found for option '-L/Users/servo/buildbot/slave/mac2/build/ports/cef/lib/x86_64-apple-darwin'
ld: warning: directory not found for option '-L/Users/servo/.rust/lib/x86_64-apple-darwin'
duplicate symbol _TaskBasicInfoVirtualSize in:
    /var/folders/sd/sks3rs2s3pb9bdrg9ldf9d5w0000gp/T/rustc.pqY5S1Luk7Wa/libtask_info-6ccbc3024e2756cc.rlib(r-task_info-task_info.o)
    /var/folders/sd/sks3rs2s3pb9bdrg9ldf9d5w0000gp/T/rustc.pqY5S1Luk7Wa/libtask_info-6ccbc3024e2756cc.rlib(r-1-task_info-task_info.o)
duplicate symbol _TaskBasicInfoResidentSize in:
    /var/folders/sd/sks3rs2s3pb9bdrg9ldf9d5w0000gp/T/rustc.pqY5S1Luk7Wa/libtask_info-6ccbc3024e2756cc.rlib(r-task_info-task_info.o)
    /var/folders/sd/sks3rs2s3pb9bdrg9ldf9d5w0000gp/T/rustc.pqY5S1Luk7Wa/libtask_info-6ccbc3024e2756cc.rlib(r-1-task_info-task_info.o)
duplicate symbol __Z8SkDebugfPKcz in:
    /var/folders/sd/sks3rs2s3pb9bdrg9ldf9d5w0000gp/T/rustc.pqY5S1Luk7Wa/libazure-f792f70de9cc89e8.rlib(r-skia-SkDebug_stdio.o)
    /var/folders/sd/sks3rs2s3pb9bdrg9ldf9d5w0000gp/T/rustc.pqY5S1Luk7Wa/libazure-f792f70de9cc89e8.rlib(r-1-skia-SkDebug_stdio.o)
ld: 3 duplicate symbols for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
@pcwalton
Copy link
Contributor

pcwalton commented Jul 11, 2015

<pcwalton> SimonSapin: sometimes the Skia one is the result of Skia and Azure not being compiled with the same -D preprocessor flags
<pcwalton> SimonSapin: I've seen that sort of thing happen with SK_DEBUG being set to 1 in skia-sys and 0 in rust-azure, or vice-versa
@pcwalton
Copy link
Contributor

pcwalton commented Jul 11, 2015

Maybe we have multiple versions of packages. Check the Cargo.lock?

@SimonSapin
Copy link
Member Author

SimonSapin commented Jul 11, 2015

The only crates duplicated in Cargo.lock are android_glue and bitflags. I couldn’t reproduce locally on a mac. Let’s see if this is intermittent.

@bors-servo retry

@bors-servo
Copy link
Contributor

bors-servo commented Jul 11, 2015

Testing commit bbed054 with merge 21433eb...

bors-servo pushed a commit that referenced this pull request Jul 11, 2015
Upgrade to rustc 1.3.0-dev (fddfd089b 2015-07-10)

Depends on mozilla/webdriver-rust#12.

Fixes #6020.

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

bors-servo commented Jul 11, 2015

💔 Test failed - mac2

@SimonSapin
Copy link
Member Author

SimonSapin commented Jul 11, 2015

Ah, I missed that this is in build-cef. I can reproduce locally.

@larsbergstrom
Copy link
Contributor

larsbergstrom commented Jul 14, 2015

Hrm. This is super confusing because when you do a build-cef, the only new things compiled are the servo crate (with a different set of features enabled) and the embedding crate.

@larsbergstrom
Copy link
Contributor

larsbergstrom commented Jul 14, 2015

Happens even on a clean build of CEF - https://pastebin.mozilla.org/8839475

@larsbergstrom
Copy link
Contributor

larsbergstrom commented Jul 14, 2015

@alexcrichton Have you ever seen something like this? We're getting duplicate symbols reported during link from a couple of crates (Skia and task-info) when we build a shared library (the CEF embedding stuff) but not when we build a binary (the final servo binary) - despite the fact that these crates are not rebuilt and we share target directories.

If I nm -g libtaskinfo, I do get what appears to be two copies of the symbol:

./deps/libtask_info-4f894c0f5635aa99.rlib
                 U _TaskBasicInfoVirtualSize
0000000000000000 T _TaskBasicInfoVirtualSize
0000000000000000 T _TaskBasicInfoVirtualSize

Working on it, but this is currently blocking our rustup, so if you have any ideas, they'd be appreciated :-)

The task_info sources are much easier to dig into than Skia: https://github.com/servo/servo/tree/rustup_2015-07-10/support/rust-task_info

@alexcrichton
Copy link
Contributor

alexcrichton commented Jul 14, 2015

I suspect this is caused by rust-lang/rust#26869 (the -force_load flag for OSX archives), although it looks like the task_info library may be getting linked twice? For example these two object filenames show up:

r-task_info-task_info.o
r-1-task_info-task_info.o

I believe that this is linking the library once and this is linking it again, so if you remove the #[link] annotation you should be good to go.

Perhaps the skia bug is also somewhat related?

@larsbergstrom
Copy link
Contributor

larsbergstrom commented Jul 14, 2015

@alexcrichton That fixed it for the task info - thanks a ton!

I suspect it's a little more complicated for skia, as the static lib is pulled in via #[link] lines in multiple places in rust-azure plus one in the actual skia repo, but I should (hopefully) be able to track down what we should really be doing. There are hacks on top of hacks dating back to partial attempts to stand something up on Android, where linking/loading was... iffy... for a while.

@alexcrichton
Copy link
Contributor

alexcrichton commented Jul 14, 2015

The pattern that I would suggest is the similar foo and foo-sys crates that crates.io has. For example if you have skia bindings I'd put them in the skia crate but the FFI definitions are in the skia-sys crate. Additionally, the #[link] (or linage via a build script) is only present for the skia-sys crate, so skia-sys is the one source of truth for linking the native library. That way the rust-azure crate could depend on skia-sys to be sure to link the library but doesn't necessarily need to pull in all the bindings of skia

@larsbergstrom
Copy link
Contributor

larsbergstrom commented Jul 14, 2015

@alexcrichton Yeah, I think most of our hacks are also to preserve link order, which is really finicky here. e.g., we need to link in fontconfig after skia or the skia bindings to FC functions will be unresolved.

@metajack
Copy link
Contributor

metajack commented Jul 14, 2015

@alexcrichton I think that's what we tried to achieve, but some of this is pre-Cargo legacy. We'll clean it up after this round of Rust upgrades and see if we run into further issues.

@bors-servo
Copy link
Contributor

bors-servo commented Jul 14, 2015

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

@SimonSapin
Copy link
Member Author

SimonSapin commented Jul 14, 2015

I’ll rebase.

@SimonSapin SimonSapin force-pushed the rustup_2015-07-10 branch from bbed054 to 1e951a1 Jul 14, 2015
@SimonSapin
Copy link
Member Author

SimonSapin commented Jul 14, 2015

I force-pushed my rebase of bbed054 without thinking of fetching first, so I overwrote any change made since. Sorry! @larsbergstrom I think you had some, could you push your local branch again? Either force-push and I can rebase again, or push to another branch and I can cherry-pick your changes.

Also, it looks like rust-layers now need to be updated for changes in Skia: https://travis-ci.org/servo/rust-layers/jobs/70484266

@SimonSapin SimonSapin force-pushed the rustup_2015-07-10 branch from 1e951a1 to 96d8fee Jul 14, 2015
GLRasterizationContext is now responsible for doing GPU rasterization.
It can coexist with its target NativeSurface, so we don't have to
continually recreate NativeSurfaces when doing GPU rasterization.
@mrobinson
Copy link
Member

mrobinson commented Jul 15, 2015

I've added the commit that gets rust-layers up to date, as well as reworking the code to work properly with it.

@pcwalton
Copy link
Contributor

pcwalton commented Jul 15, 2015

LGTM.

@larsbergstrom
Copy link
Contributor

larsbergstrom commented Jul 15, 2015

@bors-servo
Copy link
Contributor

bors-servo commented Jul 15, 2015

📌 Commit efdf2cd has been approved by larsbergstrom

@bors-servo
Copy link
Contributor

bors-servo commented Jul 15, 2015

Testing commit efdf2cd with merge 9af229b...

bors-servo pushed a commit that referenced this pull request Jul 15, 2015
Upgrade to rustc 1.3.0-dev (fddfd089b 2015-07-10)

Depends on mozilla/webdriver-rust#12.

Fixes #6020.

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

bors-servo commented Jul 15, 2015

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

@bors-servo bors-servo merged commit efdf2cd into master Jul 15, 2015
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@SimonSapin SimonSapin deleted the rustup_2015-07-10 branch Jul 15, 2015
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

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