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 core foundation #19759

Merged
merged 3 commits into from Jan 29, 2018
Merged

Upgrade core foundation #19759

merged 3 commits into from Jan 29, 2018

Conversation

@faern
Copy link
Contributor

faern commented Jan 13, 2018

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:

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.


  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes do not require tests because they don't change any code, just upgrade dependencies.

This change is Reviewable

@highfive
Copy link

highfive commented Jan 13, 2018

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @mbrubeck (or someone else) soon.

@highfive
Copy link

highfive commented Jan 13, 2018

Heads up! This PR modifies the following files:

@highfive
Copy link

highfive commented Jan 13, 2018

warning Warning warning

  • These commits modify gfx code, but no tests are modified. Please consider adding a test!
@bors-servo
Copy link
Contributor

bors-servo commented Jan 15, 2018

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

@faern faern force-pushed the faern:upgrade-core-foundation branch from 61472f1 to 81faa44 Jan 15, 2018
@bors-servo
Copy link
Contributor

bors-servo commented Jan 17, 2018

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

@faern faern force-pushed the faern:upgrade-core-foundation branch from 81faa44 to fc7e886 Jan 29, 2018
@faern
Copy link
Contributor Author

faern commented Jan 29, 2018

@jrmuizel Awesome work on the CFArray iterator. Made it possible to simplify and get rid of unsafe here as well.

@faern
Copy link
Contributor Author

faern commented Jan 29, 2018

@jdm @glennw This should hopefully be ready for merge finally!
The taskcluster says it fails, but I can't find the log for it.

@jdm
Copy link
Member

jdm commented Jan 29, 2018

@bors-servo r+
Whee!

@bors-servo
Copy link
Contributor

bors-servo commented Jan 29, 2018

📌 Commit c23534c has been approved by jdm

@highfive highfive assigned jdm and unassigned mbrubeck Jan 29, 2018
@bors-servo
Copy link
Contributor

bors-servo commented Jan 29, 2018

Testing commit c23534c with merge e94a259...

bors-servo added a commit 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 -->
@faern
Copy link
Contributor Author

faern commented Jan 29, 2018

The test that fails in travis (test_redirected_request_to_devtools) works on my machine. We'll see what homu says.

@jdm
Copy link
Member

jdm commented Jan 29, 2018

That's #14774.

@bors-servo
Copy link
Contributor

bors-servo commented Jan 29, 2018

@bors-servo bors-servo merged commit c23534c into servo:master Jan 29, 2018
2 of 4 checks passed
2 of 4 checks passed
Taskcluster (pull_request) TaskGroup: failure
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
@bors-servo bors-servo mentioned this pull request Jan 29, 2018
@faern
Copy link
Contributor Author

faern commented Jan 29, 2018

Yay, finally! Thank you @jdm for your feedback and help getting this through :)

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

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