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

Reference type as associated value instead of generic #132

Merged
merged 7 commits into from Jan 25, 2018

Conversation

faern
Copy link
Contributor

@faern faern commented Nov 24, 2017

Each CFFoo has exactly one CFFooRef and which one is defined by CFFoo, not by its user. Thus the type of reference can be an associated type instead of a generic.

As you can see this greatly simplifies things such as calling instance_of and from_CFType_pairs as well as using CFPropertyListSubClass. It basically removes one generic parameter from any method dealing with TCFTypes in some way.

This is a breaking change, so it would probably mean a 0.5.0 release if accepted. I don't know what your stance on breaking changes or version bumps is, so I don't know if you want this or not. But it would simplify the API, both here, but first and foremost in higher level crates using this.


This change is Reviewable

self.contains_key(key.as_concrete_TypeRef() as *const c_void)
pub fn contains_key2<K: TCFType>(&self, key: &K) -> bool {
let type_ref_ptr = &key.as_concrete_TypeRef() as *const _ as *const *const c_void;
self.contains_key(unsafe { *type_ref_ptr })
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implementation had to pay with a bit complexity for the reduced API complexity.

Basically, now I don't know that K::TypeRef is a *const _ any longer, so I can't just cast the pointer directly. Instead I have to take the pointer to the pointer and then cast that and lastly dereference.

I added a unit test to make sure I got it right.

let ptr = &self.0 as *const _ as *const <T as TCFType>::TypeRef;
unsafe {
let reference: &<T as TCFType>::TypeRef = &*ptr;
Some(T::wrap_under_get_rule(mem::transmute_copy(reference)))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implementation became uglier in order to get rid of the Raw generic. Using mem::transmute_copy is not very recommended maybe, but I hope this is used correctly. Here reference is of type &CFFooRef, which after the transmute_copy should be a CFFooRef. So the transmute_copy don't change the type, it just helps me take ownership of a dereferenced raw pointer. Which should be safe here unless I'm mistaken.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a more elegant solution is possible? I would be eager to learn a better way if there is one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is of course depending on the assumption that all TCFType::TypeRefs have the same size as *const c_void. This is true for all the possible subclasses of CFPropertyList at the moment and probably forever, but nothing is enforcing it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I add a second associated type, TCFType::RawTypeRef (or similar name), that we set to the corresponding __CFFoo type. Then I can refer to *const Self::RawTypeRef in places and do proper casts. From some early tinkering around it looks like that can get rid of all these strange solutions.

assert!(propertylist.downcast::<_, CFString>().unwrap().to_string() == "Bar");
assert!(propertylist.downcast::<_, CFBoolean>().is_none());
assert!(propertylist.downcast::<CFString>().unwrap().to_string() == "Bar");
assert!(propertylist.downcast::<CFBoolean>().is_none());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here one can see one of the clear benefits of this PR.

@bors-servo
Copy link
Contributor

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

@faern
Copy link
Contributor Author

faern commented Nov 26, 2017

Now I'm much more satisfied with the solution to this. By adding a trait to the sys crate that provide helper functions for const raw pointers (all CFref-types) I'm now able to know more about TCFType::ConcreteTypeRef and thus perform conversion without any transmute_copy or other ugly pointer tricks.

@faern
Copy link
Contributor Author

faern commented Nov 26, 2017

I have investigated a little bit how much this change would break published crates depending on the 0.4 version of core-foundation. I tried the following with the following results:

No change required:

  • cocoa_image
  • core-graphics
  • glutin
  • offscreen_gl_context

Some change required, but very little:

  • core-text
  • io-surface
  • font-loader

@@ -15,7 +15,7 @@ use string::CFStringRef;
#[repr(C)]
pub struct __CFError(c_void);

pub type CFErrorRef = *mut __CFError;
pub type CFErrorRef = *const __CFError;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this was *mut for a reason I have missed. But it was the only CF-ref typedef which was not *const so my guess is it was a typo. All tests of this crate and all dependant crates I tried work fine after this change.

@bors-servo
Copy link
Contributor

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

Copy link
Member

@jdm jdm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice improvement!

@faern
Copy link
Contributor Author

faern commented Nov 29, 2017

@jdm Indeed. I like how much it simplifies. But @nox did not like that it breaks stuff, which it does. However updating the dependencies to work again is quite easy.

@nox
Copy link
Contributor

nox commented Nov 29, 2017

However updating the dependencies to work again is quite easy.

It is easy but very fastidious, so I'll not let this PR land without all the others being ready too, including a PR to update them in Servo.

@faern
Copy link
Contributor Author

faern commented Nov 29, 2017

@nox If it's released as 0.5.0 merging and publishing would not break anything because of semver. But I fully understand that you don't want to end up in a messy situation where only a few things are updated.

@nox
Copy link
Contributor

nox commented Nov 29, 2017

@nox If it's released as 0.5.0 merging and publishing would not break anything because of semver. But I fully understand that you don't want to end up in a messy situation where only a few things are updated.

The issue arises when we start needing things that are unrelated to your changes, and 0.5.0 didn't land yet in servo/servo, so we must continue to maintain a 0.4 branch to get the unrelated changes in tree.

Of course, we should give you a hand and not tell you to do all the bumps.

@faern
Copy link
Contributor Author

faern commented Nov 29, 2017

@nox I'll be happy to help upgrade the dependencies! But I'm not sure which they are. All the ones I can see are the ones listed under https://crates.io/crates/core-foundation/reverse_dependencies But I suspect that is not all of it.

If you list them here I can start submitting PRs to them. Given that you want this feature that is.

@nox
Copy link
Contributor

nox commented Nov 29, 2017

Looking at the 0.2 to 0.3 bump commit gives an idea what to do.

@faern
Copy link
Contributor Author

faern commented Nov 29, 2017

@nox @jdm What are your thoughts on the name ConcreteTypeRef? I think it's long and a bit redundant. "Concrete" is already implicit by the fact that each type must state the concrete type when they implement the trait. Having "Type" in a type is kind of a special case here where we mimic names from another API that we can't control, so not that bad, but works without IMO.

How about just calling it Ref? TCFType::Ref, or in another example context: <T as TCFType>::Ref. Would be more ergonomic to use and IMO conveys the same information.

If not, how about getting rid of "Concrete" and calling it TypeRef?

@jrmuizel
Copy link
Collaborator

@nox, @jdm ping

@faern
Copy link
Contributor Author

faern commented Dec 28, 2017

@jrmuizel I didn't have time to help update servo and all dependencies yet. I still have some hope I will get to that though.

Copy link
Member

@jdm jdm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like these changes. Sorry that they sat around for so long!

fn instance_of<OtherConcreteTypeRef,OtherCFType:TCFType<OtherConcreteTypeRef>>(&self) -> bool {
self.type_of() == <OtherCFType as TCFType<_>>::type_id()
fn instance_of<OtherCFType: TCFType>(&self) -> bool {
self.type_of() == <OtherCFType as TCFType>::type_id()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be able to use OtherCFType::type_id() now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

) -> bool {
self.type_of() == <OtherCFType as TCFType<_>>::type_id()
pub fn instance_of<OtherCFType: TCFType>(&self) -> bool {
self.type_of() == <OtherCFType as TCFType>::type_id()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be able to do OtherCFType::type_id() now.

pub fn downcast<T: CFPropertyListSubClass>(&self) -> Option<T> {
if self.instance_of::<T>() {
unsafe {
let subclass_ref = <T as TCFType>::Ref::from_void_ptr(self.0);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should be able to do T::Ref::from_void_ptr(self.0).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@@ -80,6 +80,23 @@ pub struct CFAllocatorContext {
pub preferredSize: CFAllocatorPreferredSizeCallBack
}

/// Trait for all types which are Core Foundation reference types.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this commit should be reordered to come before 3688803.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. With "this commit" I assume you mean 0fc6b62 ? If so, it does come before 3688803

@faern
Copy link
Contributor Author

faern commented Jan 10, 2018

@jdm My macOS dev environment was borked, so I could not build Servo. Thus I never came around to upgrade the dependencies from Servo down to here. Would be awesome to get it in though, not sure how to fix that dev env really...

@faern
Copy link
Contributor Author

faern commented Jan 11, 2018

The travis build fails with a serious crash. It might be some bug I introduced, but it might also be something with Travis, since I can't reproduce it locally. Before I dive into this problem, does anyone know if this has show up before and/or is a known problem with this crate, or if it's something I likely introduced? @jdm ?

@faern faern mentioned this pull request Jan 11, 2018
2 tasks
@@ -92,11 +92,6 @@ impl<T> CFArray<T> {
}
}

#[deprecated(note = "please use `as_untyped` instead")]
pub fn to_untyped(self) -> CFArray {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realize now that my previous move to deprecate to_untyped and add as_untyped was not exactly making things more correct than before, if one should follow the API guidelines to the letter.
This removed to_untyped is a consuming convert (owned -> owned (non-Copy types)), so it should have been named into_untyped with that signature. So the old name was slightly "wrong".
On the other hand, the new as_untyped only borrows &self, but gives out an owned object (borrowed -> owned (non-Copy types)), so it should be named to_untyped, and not as_ since that is for conversions borrowed -> borrowed.

Do you think I should change the name of the method that will be present in the 0.5.0 release? Or is that too much nitpicking and too much changing the API back and forth? If it should be changed this would be the correct signature:

pub fn to_untyped(&self) -> CFArray { ... }

In line with all the new downcast_into methods, we could also add a consuming one that avoids bumping the retain count:

pub fn into_untyped(self) -> CFArray { ... }

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those seem like worthwhile changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check. I'll add a commit for that. I'll push it after my other PR is merged, as this one will need rebasing/manual conflict resolution after that one anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@bors-servo
Copy link
Contributor

🔒 Merge conflict

@bors-servo
Copy link
Contributor

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

@jdm
Copy link
Member

jdm commented Jan 24, 2018

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 7e87a9d has been approved by jdm

@bors-servo
Copy link
Contributor

⌛ Testing commit 7e87a9d with merge ec660b0...

bors-servo pushed a commit that referenced this pull request Jan 24, 2018
Reference type as associated value instead of generic

Each `CFFoo` has exactly one `CFFooRef` and which one is defined by `CFFoo`, not by its user. Thus the type of reference can be an associated type instead of a generic.

As you can see this greatly simplifies things such as calling `instance_of` and `from_CFType_pairs` as well as using `CFPropertyListSubClass`. It basically removes one generic parameter from any method dealing with `TCFType`s in some way.

This is a breaking change, so it would probably mean a `0.5.0` release if accepted. I don't know what your stance on breaking changes or version bumps is, so I don't know if you want this or not. But it would simplify the API, both here, but first and foremost in higher level crates using this.

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

☀️ Test successful - status-travis
Approved by: jdm
Pushing ec660b0 to master...

@bors-servo bors-servo merged commit 7e87a9d into servo:master Jan 25, 2018
@faern
Copy link
Contributor Author

faern commented Jan 25, 2018

Finally! \o/

bors-servo pushed a commit to servo/core-graphics-rs that referenced this pull request Jan 26, 2018
Upgrade core foundation

This is a PR in a series of PRs originating at servo/core-foundation-rs#132

The plan is to make a breaking change to `core-foundation` and release it as `0.5.0`. But before the merge/publish of `core-foundation` I will prepare a set of PRs making sure the entire dependency graph of Servo is ready for this change and can be switched over to `0.5.0` directly.

TODO before merge:
- [x] Merge `core-foundation` PR and publish.
- [x] Remove the last commit from this PR, so we depend on `core-foundation` from crates.io.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/core-graphics-rs/110)
<!-- Reviewable:end -->
bors-servo pushed a commit to servo/core-text-rs that referenced this pull request Jan 27, 2018
Upgrade core foundation

This is a PR in a series of PRs originating at servo/core-foundation-rs#132

The plan is to make a breaking change to `core-foundation` and release it as `0.5.0`. But before the merge/publish of `core-foundation` I will prepare a set of PRs making sure the entire dependency graph of Servo is ready for this change and can be switched over to `0.5.0` directly

TODO before merge:
- [x] Merge `core-foundation` PR and publish.
- [x] Merge `core-graphics` PR and publish.
- [x] Remove the last commit from this PR, so we depend on `core-foundation` from crates.io.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/core-text-rs/75)
<!-- Reviewable:end -->
bors-servo pushed a commit to servo/cocoa-rs that referenced this pull request Jan 27, 2018
Upgrade core foundation

This is a PR in a series of PRs originating at servo/core-foundation-rs#132

The plan is to make a breaking change to `core-foundation` and release it as `0.5.0`. But before the merge/publish of `core-foundation` I will prepare a set of PRs making sure the entire dependency graph of Servo is ready for this change and can be switched over to `0.5.0` directly.

TODO before merge:
- [x] Merge `core-graphics` PR and publish.
- [x] Remove the last commit from this PR, so we depend on `core-foundation` from crates.io.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/cocoa-rs/181)
<!-- Reviewable:end -->
bors-servo pushed a commit to servo/io-surface-rs that referenced this pull request Jan 27, 2018
Upgrade core foundation

This is a PR in a series of PRs originating at servo/core-foundation-rs#132

The plan is to make a breaking change to `core-foundation` and release it as `0.5.0`. But before the merge/publish of `core-foundation` I will prepare a set of PRs making sure the entire dependency graph of Servo is ready for this change and can be switched over to `0.5.0` directly.

TODO before merge:
- [x] Merge `core-foundation` PR and publish.
- [x] Remove the last commit from this PR, so we depend on code from crates.io.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/io-surface-rs/60)
<!-- Reviewable:end -->
bors-servo pushed a commit to servo/glutin that referenced this pull request Jan 27, 2018
Upgrade core foundation

This is a PR in a series of PRs originating at servo/core-foundation-rs#132

The plan is to make a breaking change to `core-foundation` and release it as `0.5.0`. But before the merge/publish of `core-foundation` I will prepare a set of PRs making sure the entire dependency graph of Servo is ready for this change and can be switched over to `0.5.0` directly.

TODO before merge:
- [x] Merge `core-foundation`, `core-graphics` and `cocoa` PRs and publish.
- [x] Remove the last commit from this PR, so we depend on code from crates.io.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/glutin/142)
<!-- Reviewable:end -->
bors-servo pushed a commit to servo/skia that referenced this pull request Jan 27, 2018
Upgrade core foundation

This is a PR in a series of PRs originating at servo/core-foundation-rs#132

The plan is to make a breaking change to `core-foundation` and release it as `0.5.0`. Before the merge/publish, a set of PRs making sure the entire dependency graph of Servo is ready for this change will be created.

TODO before merge of this PR:
- [x] Merge `io-surface` and `servo-glutin` PRs and publish.
- [x] Remove the last commit from this PR, so we depend on code from crates.io.

I did not read the contributing guidelines for this repo, as the link to it did not work for me.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/skia/148)
<!-- Reviewable:end -->
bors-servo pushed a commit to servo/rust-azure that referenced this pull request Jan 28, 2018
Upgrade core foundation

This is a PR in a series of PRs originating at servo/core-foundation-rs#132

The plan is to make a breaking change to `core-foundation` and release it as `0.5.0`. Hopefully we can manage to bring Servo and its entire dependency tree up to date as rapidly as possible in combination with this, as to use only the new core-foundation in Servo and all its deps. :)

TODO before merge:
- [x] Merge `skia`.
- [x] Remove the last commit from this PR, so we depend on code from crates.io.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-azure/282)
<!-- Reviewable:end -->
@faern faern deleted the tcftype-ref-as-associated branch January 28, 2018 23:33
bors-servo pushed a commit to servo/webrender that referenced this pull request Jan 29, 2018
Upgrade core foundation

This is a PR in a series of PRs originating at servo/core-foundation-rs#132

The plan is to make a breaking change to `core-foundation` and release it as `0.5.0`. Hopefully we can manage to bring Servo and its entire dependency tree up to date as rapidly as possible in combination with this, as to use only the new core-foundation in Servo and all its deps. :)

TODO before merge:
- [x] Merge `core-foundation`, `core-graphics`, `core-text`, `servo-glutin` PRs and publish.
- [x] Merge `font-loader` PR and publish.
- [x] Remove the last commit from this PR, so we depend on code from crates.io.
- [x] Update Cargo.lock again, to not contain urls to temporary feature branhces.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/2299)
<!-- Reviewable:end -->
bors-servo pushed a commit to servo/servo that referenced this pull request Jan 29, 2018
Upgrade core foundation

<!-- Please describe your changes on the following line: -->
This PR is the final one in a chain of PRs that tries to make a breaking change to `core-foundation`. This PR makes sure Servo only use the new, not yet released `core-foundation 0.5.0`. The changes in `core-foundation` and why it is not yet published can be read in the comments on this PR: servo/core-foundation-rs#132
Basically we want all of Servo (and deps) to be ready for a fairly swift upgrade from `core-foundation` `0.4.6` to `0.5.0` once it's released, so we don't end up in some state where we depend on, and have to maintain both, for an extended period of time.

This PR is **not ready for merge** in its current state. The following must be done first:
- [x] Merge servo/core-foundation-rs#132 and publish.
- [x] Merge servo/core-graphics-rs#110 and publish.
- [x] Merge servo/core-text-rs#75 and publish.
- [x] Merge servo/cocoa-rs#181 and publish.
- [x] Merge servo/glutin#142 and publish.
- [x] Merge servo/io-surface-rs#60 and publish.
- [x] Merge servo/skia#148.
- [x] Merge servo/rust-azure#282.
- [x] Merge servo/webrender#2299.
- [x] Merge servo/surfman#118 and publish.
- [x] Remove the commit in this PR that temporarily adds patch entries to `Cargo.toml`.
- [x] Update Cargo.lock again to not point to my feature branches.

For some of the dependencies I might accidentally have bumped the version as if it was a breaking change when it in fact wasn't. It was a bit messy to figure out all the details in so many and large crates. But hopefully I did not do the inverse, only bump the patch version where the change actually broke something.

Ping @jdm and @nox who have been the ones commenting on the initial PR.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors

<!-- Either: -->
- [X] These changes do not require tests because they don't change any code, just upgrade dependencies.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/19759)
<!-- Reviewable:end -->
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Jan 30, 2018
…e-foundation); r=jdm

<!-- Please describe your changes on the following line: -->
This PR is the final one in a chain of PRs that tries to make a breaking change to `core-foundation`. This PR makes sure Servo only use the new, not yet released `core-foundation 0.5.0`. The changes in `core-foundation` and why it is not yet published can be read in the comments on this PR: servo/core-foundation-rs#132
Basically we want all of Servo (and deps) to be ready for a fairly swift upgrade from `core-foundation` `0.4.6` to `0.5.0` once it's released, so we don't end up in some state where we depend on, and have to maintain both, for an extended period of time.

This PR is **not ready for merge** in its current state. The following must be done first:
- [x] Merge servo/core-foundation-rs#132 and publish.
- [x] Merge servo/core-graphics-rs#110 and publish.
- [x] Merge servo/core-text-rs#75 and publish.
- [x] Merge servo/cocoa-rs#181 and publish.
- [x] Merge servo/glutin#142 and publish.
- [x] Merge servo/io-surface-rs#60 and publish.
- [x] Merge servo/skia#148.
- [x] Merge servo/rust-azure#282.
- [x] Merge servo/webrender#2299.
- [x] Merge servo/surfman#118 and publish.
- [x] Remove the commit in this PR that temporarily adds patch entries to `Cargo.toml`.
- [x] Update Cargo.lock again to not point to my feature branches.

For some of the dependencies I might accidentally have bumped the version as if it was a breaking change when it in fact wasn't. It was a bit messy to figure out all the details in so many and large crates. But hopefully I did not do the inverse, only bump the patch version where the change actually broke something.

Ping @jdm and @nox who have been the ones commenting on the initial PR.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors

<!-- Either: -->
- [X] These changes do not require tests because they don't change any code, just upgrade dependencies.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: e94a25949c924e086e38ef6bdbdc935734415b26

--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : 4e0c40d1610e09d5ecf823f51d42ede8706751e3
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 2, 2019
…e-foundation); r=jdm

<!-- Please describe your changes on the following line: -->
This PR is the final one in a chain of PRs that tries to make a breaking change to `core-foundation`. This PR makes sure Servo only use the new, not yet released `core-foundation 0.5.0`. The changes in `core-foundation` and why it is not yet published can be read in the comments on this PR: servo/core-foundation-rs#132
Basically we want all of Servo (and deps) to be ready for a fairly swift upgrade from `core-foundation` `0.4.6` to `0.5.0` once it's released, so we don't end up in some state where we depend on, and have to maintain both, for an extended period of time.

This PR is **not ready for merge** in its current state. The following must be done first:
- [x] Merge servo/core-foundation-rs#132 and publish.
- [x] Merge servo/core-graphics-rs#110 and publish.
- [x] Merge servo/core-text-rs#75 and publish.
- [x] Merge servo/cocoa-rs#181 and publish.
- [x] Merge servo/glutin#142 and publish.
- [x] Merge servo/io-surface-rs#60 and publish.
- [x] Merge servo/skia#148.
- [x] Merge servo/rust-azure#282.
- [x] Merge servo/webrender#2299.
- [x] Merge servo/surfman#118 and publish.
- [x] Remove the commit in this PR that temporarily adds patch entries to `Cargo.toml`.
- [x] Update Cargo.lock again to not point to my feature branches.

For some of the dependencies I might accidentally have bumped the version as if it was a breaking change when it in fact wasn't. It was a bit messy to figure out all the details in so many and large crates. But hopefully I did not do the inverse, only bump the patch version where the change actually broke something.

Ping jdm and nox who have been the ones commenting on the initial PR.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors

<!-- Either: -->
- [X] These changes do not require tests because they don't change any code, just upgrade dependencies.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: e94a25949c924e086e38ef6bdbdc935734415b26

UltraBlame original commit: 931c61b9f7a889329ab73d00414483af47ac3f7e
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 2, 2019
…e-foundation); r=jdm

<!-- Please describe your changes on the following line: -->
This PR is the final one in a chain of PRs that tries to make a breaking change to `core-foundation`. This PR makes sure Servo only use the new, not yet released `core-foundation 0.5.0`. The changes in `core-foundation` and why it is not yet published can be read in the comments on this PR: servo/core-foundation-rs#132
Basically we want all of Servo (and deps) to be ready for a fairly swift upgrade from `core-foundation` `0.4.6` to `0.5.0` once it's released, so we don't end up in some state where we depend on, and have to maintain both, for an extended period of time.

This PR is **not ready for merge** in its current state. The following must be done first:
- [x] Merge servo/core-foundation-rs#132 and publish.
- [x] Merge servo/core-graphics-rs#110 and publish.
- [x] Merge servo/core-text-rs#75 and publish.
- [x] Merge servo/cocoa-rs#181 and publish.
- [x] Merge servo/glutin#142 and publish.
- [x] Merge servo/io-surface-rs#60 and publish.
- [x] Merge servo/skia#148.
- [x] Merge servo/rust-azure#282.
- [x] Merge servo/webrender#2299.
- [x] Merge servo/surfman#118 and publish.
- [x] Remove the commit in this PR that temporarily adds patch entries to `Cargo.toml`.
- [x] Update Cargo.lock again to not point to my feature branches.

For some of the dependencies I might accidentally have bumped the version as if it was a breaking change when it in fact wasn't. It was a bit messy to figure out all the details in so many and large crates. But hopefully I did not do the inverse, only bump the patch version where the change actually broke something.

Ping jdm and nox who have been the ones commenting on the initial PR.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors

<!-- Either: -->
- [X] These changes do not require tests because they don't change any code, just upgrade dependencies.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: e94a25949c924e086e38ef6bdbdc935734415b26

UltraBlame original commit: 931c61b9f7a889329ab73d00414483af47ac3f7e
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 2, 2019
…e-foundation); r=jdm

<!-- Please describe your changes on the following line: -->
This PR is the final one in a chain of PRs that tries to make a breaking change to `core-foundation`. This PR makes sure Servo only use the new, not yet released `core-foundation 0.5.0`. The changes in `core-foundation` and why it is not yet published can be read in the comments on this PR: servo/core-foundation-rs#132
Basically we want all of Servo (and deps) to be ready for a fairly swift upgrade from `core-foundation` `0.4.6` to `0.5.0` once it's released, so we don't end up in some state where we depend on, and have to maintain both, for an extended period of time.

This PR is **not ready for merge** in its current state. The following must be done first:
- [x] Merge servo/core-foundation-rs#132 and publish.
- [x] Merge servo/core-graphics-rs#110 and publish.
- [x] Merge servo/core-text-rs#75 and publish.
- [x] Merge servo/cocoa-rs#181 and publish.
- [x] Merge servo/glutin#142 and publish.
- [x] Merge servo/io-surface-rs#60 and publish.
- [x] Merge servo/skia#148.
- [x] Merge servo/rust-azure#282.
- [x] Merge servo/webrender#2299.
- [x] Merge servo/surfman#118 and publish.
- [x] Remove the commit in this PR that temporarily adds patch entries to `Cargo.toml`.
- [x] Update Cargo.lock again to not point to my feature branches.

For some of the dependencies I might accidentally have bumped the version as if it was a breaking change when it in fact wasn't. It was a bit messy to figure out all the details in so many and large crates. But hopefully I did not do the inverse, only bump the patch version where the change actually broke something.

Ping jdm and nox who have been the ones commenting on the initial PR.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors

<!-- Either: -->
- [X] These changes do not require tests because they don't change any code, just upgrade dependencies.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

Source-Repo: https://github.com/servo/servo
Source-Revision: e94a25949c924e086e38ef6bdbdc935734415b26

UltraBlame original commit: 931c61b9f7a889329ab73d00414483af47ac3f7e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants