Skip to content

/ servo

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

Use skia and deps from crates.io. #8757

Merged
merged 2 commits into from Dec 3, 2015
Merged

Use skia and deps from crates.io. #8757

merged 2 commits into from Dec 3, 2015

Conversation

@SimonSapin
Copy link
Member

SimonSapin commented Nov 30, 2015

This makes the initial download for skia go from a 300 MB git repository to a 5 MB tarball. This should help with issues like #6132 and #7687.

Fix servo/skia#70

This builds, but the at the moment causes a number of tidy errors for duplicated crates.

Review on Reviewable
@highfive
Copy link

highfive commented Nov 30, 2015

warning Warning warning

  • These commits modify layout code, but no reftests are modified. Please consider adding a reftest!
@metajack metajack self-assigned this Nov 30, 2015
@metajack
Copy link
Contributor

metajack commented Nov 30, 2015

This adds a lot of dupe dependencies. xml-rs only on gonk for some reason. the others seem to be on all the Cargo.locks.

Reviewed 24 of 24 files at r1.
Review status: 21 of 24 files reviewed at latest revision, 6 unresolved discussions.

components/servo/Cargo.lock, line 316 [r1] (raw file):
now we have two core-foundations. 0.1.0 is included just before this one.

components/servo/Cargo.lock, line 343 [r1] (raw file):
dupe core-graphics too

components/servo/Cargo.lock, line 723 [r1] (raw file):
two gl_generators. although this one may be intentional?

components/servo/Cargo.lock, line 856 [r1] (raw file):
two heapsizes

components/servo/Cargo.lock, line 991 [r1] (raw file):
dupe khronos apis

ports/gonk/Cargo.lock, line 2069 [r1] (raw file):
dupe xml-rsx

Comments from the review on Reviewable.io
@SimonSapin SimonSapin force-pushed the skia branch 4 times, most recently from 0333df7 to 6d92fdb Nov 30, 2015
@SimonSapin
Copy link
Member Author

SimonSapin commented Dec 1, 2015

Should be good after updating to servo/surfman#44
@SimonSapin
Copy link
Member Author

SimonSapin commented Dec 1, 2015

@bors-servo try
@bors-servo
Copy link
Contributor

bors-servo commented Dec 1, 2015

⌛ Trying commit 4a6ee53 with merge fb46d81...
bors-servo added a commit that referenced this pull request Dec 1, 2015
@bors-servo
Auto merge of #8757 - servo:skia, r=<try>
fb46d81 
Use skia and deps from crates.io.

This makes the initial download for skia go from a 300 MB git repository to a 5 MB tarball. This should help with issues like #6132 and #7687.

Fix servo/skia#70

This builds, but the at the moment causes a number of tidy errors for duplicated crates.

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

bors-servo commented Dec 1, 2015

💔 Test failed - mac-dev-ref-unit
@highfive highfive removed the S-tests-failed label Dec 1, 2015
@SimonSapin
Copy link
Member Author

SimonSapin commented Dec 1, 2015

@bors-servo try
@bors-servo
Copy link
Contributor

bors-servo commented Dec 1, 2015

⌛ Trying commit 9237ae3 with merge e0cda69...
bors-servo added a commit that referenced this pull request Dec 1, 2015
@bors-servo
Auto merge of #8757 - servo:skia, r=<try>
e0cda69 
Use skia and deps from crates.io.

This makes the initial download for skia go from a 300 MB git repository to a 5 MB tarball. This should help with issues like #6132 and #7687.

Fix servo/skia#70

This builds, but the at the moment causes a number of tidy errors for duplicated crates.

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

bors-servo commented Dec 1, 2015

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt
@SimonSapin
Copy link
Member Author

SimonSapin commented Dec 1, 2015

@metajack All of the duplicate dependencies were detected by tidy, which is now happy :)
@mbrubeck
Copy link
Contributor

mbrubeck commented Dec 2, 2015

ports/cef/Cargo.lock still has some duplicates for some reason.

Reviewed 2 of 9 files at r2.
Review status: 19 of 26 files reviewed at latest revision, 9 unresolved discussions.

ports/cef/Cargo.lock, line 244 [r2] (raw file):
Duplicate cocoa. Not sure how this one slipped past tidy.

ports/cef/Cargo.lock, line 317 [r2] (raw file):
Duplicate core-foundation.

ports/cef/Cargo.lock, line 343 [r2] (raw file):
Duplicate core-graphics.

Comments from the review on Reviewable.io
@mbrubeck
Copy link
Contributor

mbrubeck commented Dec 2, 2015

Reviewed 6 of 9 files at r2.
Review status: 25 of 26 files reviewed at latest revision, 10 unresolved discussions.

ports/cef/Cargo.toml, line 72 [r2] (raw file):
This toml file has a bunch of outdated or "*" dependencies should be updated. Or maybe they can just be removed, since it should depend on these transitively rather than directly?

Comments from the review on Reviewable.io
@SimonSapin
Copy link
Member Author

SimonSapin commented Dec 2, 2015

Ah, I was looking at ./mach test-tidy, but it seems to ignore everything under ports/cef/
@SimonSapin
Use skia and deps from crates.io.
Loading status checks…
aa1eba4 
Fix servo/skia#70
@SimonSapin
Tidy ports/cef/Cargo.lock
Loading status checks…
b8a3a64 
Despite the rest of ports/cef/ being ignored.
@mbrubeck
Copy link
Contributor

mbrubeck commented Dec 2, 2015

Reviewed 1 of 9 files at r2, 3 of 3 files at r3.
Review status: all files reviewed at latest revision, 7 unresolved discussions.

Comments from the review on Reviewable.io
@mbrubeck
Copy link
Contributor

mbrubeck commented Dec 2, 2015

@bors-servo r+
@mbrubeck
Copy link
Contributor

mbrubeck commented Dec 2, 2015

@bors-servo try- r+
@mbrubeck
Copy link
Contributor

mbrubeck commented Dec 2, 2015

@bors-servo retry
@larsbergstrom
Copy link
Contributor

larsbergstrom commented Dec 2, 2015

@bors-servo try- retry r=mbrubeck
@bors-servo
Copy link
Contributor

bors-servo commented Dec 2, 2015

📌 Commit b8a3a64 has been approved by mbrubeck
@metajack
Copy link
Contributor

metajack commented Dec 2, 2015

Ah, I was looking at ./mach test-tidy, but it seems to ignore everything under ports/cef/

Please file a bug for this! It should only be ignoring ports/cef for certain things, but definitely not for cargo.lock checks.
@SimonSapin
Copy link
Member Author

SimonSapin commented Dec 2, 2015

My second commit fixes this: b8a3a64
@metajack
Copy link
Contributor

metajack commented Dec 2, 2015

Even better!
@frewsxcv
Copy link
Member

frewsxcv commented Dec 2, 2015

Please file a bug for this! It should only be ignoring ports/cef for certain things, but definitely not for cargo.lock checks.

Relevant discussion: #7202
@bors-servo
Copy link
Contributor

bors-servo commented Dec 3, 2015

⌛ Testing commit b8a3a64 with merge 8b95d7b...
bors-servo added a commit that referenced this pull request Dec 3, 2015
@bors-servo
Auto merge of #8757 - servo:skia, r=mbrubeck
Loading status checks…
8b95d7b 
Use skia and deps from crates.io.

This makes the initial download for skia go from a 300 MB git repository to a 5 MB tarball. This should help with issues like #6132 and #7687.

Fix servo/skia#70

This builds, but the at the moment causes a number of tidy errors for duplicated crates.

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

bors-servo commented Dec 3, 2015

☀️ Test successful - android, gonk, linux-dev, linux-rel, mac-dev-ref-unit, mac-rel-css, mac-rel-wpt
@bors-servo bors-servo merged commit b8a3a64 into master Dec 3, 2015
2 of 3 checks passed
2 of 3 checks passed
code-review/reviewable Review in progress: all files reviewed, 7 unresolved discussions
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
@SimonSapin SimonSapin deleted the skia branch Dec 3, 2015
@SimonSapin SimonSapin mentioned this pull request Dec 4, 2015
Closed
@benbucksch
Copy link

benbucksch commented Dec 4, 2015

Thank you
@SimonSapin SimonSapin mentioned this pull request Dec 4, 2015
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@metajack metajack

Labels
S-awaiting-merge
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

8 participants
@SimonSapin @highfive @metajack @bors-servo @mbrubeck @larsbergstrom @frewsxcv @benbucksch
You can’t perform that action at this time.