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

Update `ipc-channel` dependency to 0.6 #14146

Closed
wants to merge 1 commit into from

Conversation

@antrik
Copy link
Contributor

antrik commented Nov 9, 2016

Pull in the new interface, to make sure nobody accidentally breaks
compatibility with ipc_channel::Sender no longer being Sync.

This also requires pulling in a new webrender, to get the updated
ipc-channel dependency there as well.


This change is Reviewable

@highfive
Copy link

highfive commented Nov 9, 2016

Heads up! This PR modifies the following files:

  • @asajeffrey: components/webdriver_server/Cargo.toml, components/constellation/Cargo.toml
  • @jgraham: components/webdriver_server/Cargo.toml
  • @KiChjang: components/script/Cargo.toml, components/script_traits/Cargo.toml, components/script_traits/Cargo.toml, components/net/Cargo.toml, components/script_layout_interface/Cargo.toml, components/net_traits/Cargo.toml, components/net_traits/Cargo.toml
  • @fitzgen: components/devtools_traits/Cargo.toml, components/devtools_traits/Cargo.toml, components/profile/Cargo.toml, components/profile_traits/Cargo.toml, components/profile_traits/Cargo.toml, components/devtools/Cargo.toml, components/script/Cargo.toml, components/script_traits/Cargo.toml, components/script_traits/Cargo.toml, components/script_layout_interface/Cargo.toml
  • @emilio: components/layout/Cargo.toml
@nox
Copy link
Member

nox commented Nov 9, 2016

error[E0061]: this function takes 5 parameters but 4 parameters were supplied
   --> /home/travis/build/servo/servo/components/net/image_cache_thread.rs:326:45
    |
326 |     image.id = Some(webrender_api.add_image(image.width, image.height, format, bytes));
    |                                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: the following parameter types were expected:
    = note: u32, u32, std::option::Option<u32>, webrender_traits::ImageFormat, std::vec::Vec<u8>
error[E0061]: this function takes 5 parameters but 4 parameters were supplied
   --> /home/travis/build/servo/servo/components/net/image_cache_thread.rs:481:62
    |
481 |                 image.id = Some(self.webrender_api.add_image(image.width, image.height, format, bytes));
    |                                                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: the following parameter types were expected:
    = note: u32, u32, std::option::Option<u32>, webrender_traits::ImageFormat, std::vec::Vec<u8>
error: aborting due to 2 previous errors
@nox nox assigned nox and unassigned cbrewster Nov 9, 2016
@glennw
Copy link
Member

glennw commented Nov 9, 2016

@nox @antrik There's a pending WR update in the queue that updates WR to just before the ipc-channel bump, and includes a compile fix for the error above - #14145

@bors-servo
Copy link
Contributor

bors-servo commented Nov 9, 2016

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

Pull in the new interface, to make sure nobody accidentally breaks
compatibility with `ipc_channel::Sender` no longer being `Sync`.

This also requires pulling in a new `webrender`, to get the updated
`ipc-channel` dependency there as well.
@antrik antrik force-pushed the antrik:update-ipc_channel-6 branch from a91681b to 6aad579 Nov 9, 2016
@antrik
Copy link
Contributor Author

antrik commented Nov 9, 2016

Weird, it worked fine in my local testing... Guess I must have mixed up the versions or something.

Anyway, I rebased it now -- that should fix it I think?

@emilio
Copy link
Member

emilio commented Nov 9, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Nov 9, 2016

📌 Commit 6aad579 has been approved by emilio

@bors-servo
Copy link
Contributor

bors-servo commented Nov 10, 2016

Testing commit 6aad579 with merge f9c178b...

bors-servo added a commit that referenced this pull request Nov 10, 2016
Update `ipc-channel` dependency to 0.6

Pull in the new interface, to make sure nobody accidentally breaks
compatibility with `ipc_channel::Sender` no longer being `Sync`.

This also requires pulling in a new `webrender`, to get the updated
`ipc-channel` dependency there as well.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14146)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Nov 10, 2016

💔 Test failed - linux-rel-wpt

@highfive
Copy link

highfive commented Nov 10, 2016

  ▶ FAIL [expected PASS] /_mozilla/css/incremental_trailing_whitespace_a.html
  └   → /_mozilla/css/incremental_trailing_whitespace_a.html da54041086f2975f8e3c776d2283ad6609e6862a
/_mozilla/css/incremental_trailing_whitespace_ref.html f5d0147c8bddbdc772a7e5e86c7c9e433fcd486b
Testing da54041086f2975f8e3c776d2283ad6609e6862a == f5d0147c8bddbdc772a7e5e86c7c9e433fcd486b
@glennw
Copy link
Member

glennw commented Nov 10, 2016

@bors-servo
Copy link
Contributor

bors-servo commented Nov 10, 2016

Testing commit 6aad579 with merge fdce57a...

bors-servo added a commit that referenced this pull request Nov 10, 2016
Update `ipc-channel` dependency to 0.6

Pull in the new interface, to make sure nobody accidentally breaks
compatibility with `ipc_channel::Sender` no longer being `Sync`.

This also requires pulling in a new `webrender`, to get the updated
`ipc-channel` dependency there as well.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14146)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Nov 10, 2016

💔 Test failed - mac-rel-wpt2

@glennw
Copy link
Member

glennw commented Nov 10, 2016

@antrik That last failure looks like a genuine ipc-channel issue?

Tests with unexpected results:
  ▶ TIMEOUT [expected PASS] /_mozilla/css/data_img_a.html
  │ 
  │ VMware, Inc.
  │ Gallium 0.4 on softpipe
  │ 3.3 (Core Profile) Mesa 12.0.1
  │ message was bigger than we were told (thread <unnamed>, at /Users/servo/.cargo/registry/src/github.com-1ecc6299db9ec823/ipc-channel-0.6.0/src/platform/macos/mod.rs:598)
  │ stack backtrace:
  │    0:        0x10518284e - backtrace::backtrace::trace::h16372ee7bf1517e5
  │    1:        0x105182b3c - backtrace::capture::Backtrace::new::hcc43c50c4b11c693
  │    2:        0x10402b744 - servo::main::_{{closure}}::h356eefa3f34c1614
  │    3:        0x105a1f4d3 - std::panicking::rust_panic_with_hook::h00b81bb3dcbd51f2
  │    4:        0x105933a54 - std::panicking::begin_panic::h70ee11343cda1bc2
  │    5:        0x105940cf8 - ipc_channel::platform::os::select::h7ea4b4984f1d3d65
  │    6:        0x10593c036 - ipc_channel::ipc::IpcReceiverSet::select::h72ac7c6016f374cb
  │    7:        0x105938bbb - _<std..panic..AssertUnwindSafe<F> as core..ops..FnOnce<()>>::call_once::h4d3168353c8100fa
  │    8:        0x105933a87 - std::panicking::try::do_call::h31e6deb9d0aac73f
  │    9:        0x105a204fa - __rust_maybe_catch_panic
  │   10:        0x10593712b - _<F as alloc..boxed..FnBox<A>>::call_box::h3baf06495ae3a200
  │   11:        0x105a1e384 - std::sys::thread::Thread::new::thread_start::h9fa66f6812e81e10
  │   12:     0x7fff88879059 - _pthread_body
  │   13:     0x7fff88878fd6 - _pthread_start
  │ ERROR:servo: message was bigger than we were told
  │ called `Result::unwrap()` on an `Err` value: RecvError (thread ImageCacheThread, at ../src/libcore/result.rs:837)
  │ stack backtrace:
  │    0:        0x10518284e - backtrace::backtrace::trace::h16372ee7bf1517e5
  │    1:        0x105182b3c - backtrace::capture::Backtrace::new::hcc43c50c4b11c693
  │    2:        0x10402b744 - servo::main::_{{closure}}::h356eefa3f34c1614
  │    3:        0x105a1f4d3 - std::panicking::rust_panic_with_hook::h00b81bb3dcbd51f2
  │    4:        0x105a1f3a4 - std::panicking::begin_panic::ha6a0d553db9869ff
  │    5:        0x105a1f2c2 - std::panicking::begin_panic_fmt::h24d113aee3ee4081
  │    6:        0x105a1f227 - rust_begin_unwind
  │    7:        0x105a4bff0 - core::panicking::panic_fmt::he441b2ea2036b98a
  │    8:        0x10576a86b - core::result::unwrap_failed::h4bfe8251a66d758c
  │    9:        0x1057d8f74 - net::image_cache_thread::Receivers::recv::h3a8712eef2aa499d
  │   10:        0x1057da6ba - net::image_cache_thread::ImageCache::run::h39461411f1d54672
  │   11:        0x105751977 - std::panicking::try::do_call::hdeab639a9b109e66
  │   12:        0x105a204fa - __rust_maybe_catch_panic
  │   13:        0x10577c4fb - _<F as alloc..boxed..FnBox<A>>::call_box::hc284545e17de67c4
  │   14:        0x105a1e384 - std::sys::thread::Thread::new::thread_start::h9fa66f6812e81e10
  │   15:     0x7fff88879059 - _pthread_body
  │   16:     0x7fff88878fd6 - _pthread_start
  └ ERROR:servo: called `Result::unwrap()` on an `Err` value: RecvError

program finished with exit code 1
elapsedTime=1336.422393
@antrik
Copy link
Contributor Author

antrik commented Nov 10, 2016

@glennw yeah, there were quite some changes to ipc-channel between 0.5.1 and 0.6 -- and I fear not all of them have actually been tested with Servo... While ipc-channel has fairly good test coverage, obviously there will always be things the tests won't catch :-(

I have a strong suspicion that the culprit is servo/ipc-channel#99 -- not least because that's what introduced the error message we see here... Though of course it might be something else too. We need someone with MacOS to look into this.

@antrik
Copy link
Contributor Author

antrik commented Nov 10, 2016

(When I say culprit, I do not necessarily mean it's the root cause of the problem -- of course it might just be exposing bugs present elsewhere...)

@jdm
Copy link
Member

jdm commented Nov 10, 2016

I'll investigate.

@jdm
Copy link
Member

jdm commented Nov 10, 2016

servo/ipc-channel#119 makes the test pass.

@bors-servo
Copy link
Contributor

bors-servo commented Nov 11, 2016

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

@nox
Copy link
Member

nox commented Feb 12, 2017

The bug in ipc-channel has been fixed, but now the bump is blocked on #15467 to land first, because WR needs the ipc-channel bump too.

@antrik
Copy link
Contributor Author

antrik commented Feb 14, 2017

Superseded by #15537

@antrik antrik closed this Feb 14, 2017
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.