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

Move to Cargo workspaces #14381

Merged
merged 1 commit into from Nov 28, 2016
Merged

Move to Cargo workspaces #14381

merged 1 commit into from Nov 28, 2016

Conversation

@nox
Copy link
Member

nox commented Nov 27, 2016 •
edited by larsbergstrom

This change is Reviewable
@highfive
Copy link

highfive commented Nov 27, 2016

Heads up! This PR modifies the following files:

  • @wafflespeanut: python/servo/command_base.py, python/servo/devenv_commands.py, python/tidy/servo_tidy/tidy.py
  • @emilio: ports/geckolib/Cargo.lock, ports/geckolib/Cargo.toml
@nox
Copy link
Member Author

nox commented Nov 27, 2016

Cc @larsbergstrom
@nox nox mentioned this pull request Nov 27, 2016
Closed
@SimonSapin
SimonSapin reviewed Nov 27, 2016
View changes
python/servo/devenv_commands.py
@@ -89,11 +89,9 @@ def update_cargo(self, params=None, package=None, all_packages=None):

self.ensure_bootstrapped()

for cargo_path in CARGO_PATHS:

This comment has been minimized.

Sign in to view
@SimonSapin

SimonSapin Nov 27, 2016

Member

r=me with CARGO_PATHS removed near the top of the file.
@SimonSapin
Copy link
Member

SimonSapin commented Nov 27, 2016

@larsbergstrom in #12391 (comment):

Ensure that cargo vendor on ports/geckolib does NOT pull in the full dependency graph

TL;DR: We’re good. It looks like, when a path dependency is in a workspace, that workspace is completely ignored.

mozilla-central has a single gkrust crate in its toolkit/library/rust directory that (recursively) depends on all the Rust code included in Gecko. In the incubator/stylo repository, there’s (indirectly) a path dependency to ../../../servo/ports/geckolib.

To get a similar but simplified setup, I created a temporary directory with this Cargo.toml:

[package]
name = "vendor-test"
version = "0.0.0"

[dependencies]
geckoservo = {path = "/home/simon/projects/servo2/ports/geckolib"}

… where servo2 has this PR checked out. Then I ran cargo update, cargo install cargo-vendor, and cargo vendor. This creates a Cargo.lock file that contains the line [package] 61 times, and a vendor directory that contains 55 sub-directories. (The difference is the number of recursive path dependencies.)

For comparison, /home/simon/projects/servo2/Cargo.lock contains the line [package] exactly 300 times.
@nox nox force-pushed the workspaces branch from 8c617e6 to ef287f8 Nov 27, 2016
@SimonSapin
Copy link
Member

SimonSapin commented Nov 27, 2016

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

bors-servo commented Nov 27, 2016

📌 Commit ef287f8 has been approved by SimonSapin
@highfive highfive assigned SimonSapin and unassigned glennw Nov 27, 2016
bors-servo added a commit that referenced this pull request Nov 27, 2016
@bors-servo
Auto merge of #14381 - servo:workspaces, r=SimonSapin
Loading status checks…
52f4be3 
Move to Cargo workspaces

<!-- 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/14381)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Nov 27, 2016

⌛ Testing commit ef287f8 with merge 52f4be3...
@bors-servo
Copy link
Contributor

bors-servo commented Nov 27, 2016

💔 Test failed - linux-dev
@nox
Move to Cargo workspaces
Loading status checks…
dfb35db
@nox nox force-pushed the workspaces branch from ef287f8 to dfb35db Nov 28, 2016
@nox
Copy link
Member Author

nox commented Nov 28, 2016

@bors-servo r=SimonSapin
@bors-servo
Copy link
Contributor

bors-servo commented Nov 28, 2016

📌 Commit dfb35db has been approved by SimonSapin
@nox
Copy link
Member Author

nox commented Nov 28, 2016

@bors-servo p=11
bors-servo added a commit that referenced this pull request Nov 28, 2016
@bors-servo
Auto merge of #14381 - servo:workspaces, r=SimonSapin
Loading status checks…
eb7032f 
Move to Cargo workspaces

<!-- 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/14381)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Nov 28, 2016

⌛ Testing commit dfb35db with merge eb7032f...
@bors-servo
Copy link
Contributor

bors-servo commented Nov 28, 2016

💔 Test failed - linux-rel-wpt
@nox
Copy link
Member Author

nox commented Nov 28, 2016

@bors-servo retry #13776
@bors-servo
Copy link
Contributor

bors-servo commented Nov 28, 2016

⚡ Previous build results for arm32, arm64, linux-dev, linux-rel-css, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-dev are reusable. Rebuilding only linux-rel-wpt...
@bors-servo
Copy link
Contributor

bors-servo commented Nov 28, 2016

☀️ Test successful - arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css, mac-rel-wpt1, mac-rel-wpt2, windows-dev
@bors-servo bors-servo merged commit dfb35db into master Nov 28, 2016
4 checks passed
4 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
dependency-ci Dependencies checked
Details
homu Test successful
Details
This was referenced Nov 28, 2016
Merged
Merged
Merged
Merged
Merged
@Ms2ger Ms2ger deleted the workspaces branch Nov 28, 2016
@SimonSapin SimonSapin mentioned this pull request Nov 28, 2016
Closed
@jdm jdm mentioned this pull request Nov 29, 2016
Merged
bors-servo added a commit that referenced this pull request Nov 30, 2016
@bors-servo
Auto merge of #14413 - servo:jdm-patch-1, r=mbrubeck
Loading status checks…
5a07f2d 
Remove misleading section from non-toplevel Cargo.toml.

This made me waste some time trying to figure out why my builds were no longer getting debug information after #14381.

<!-- 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/14413)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this pull request Nov 30, 2016
@bors-servo
Auto merge of #14413 - servo:jdm-patch-1, r=mbrubeck
Loading status checks…
7257ffe 
Remove misleading section from non-toplevel Cargo.toml.

This made me waste some time trying to figure out why my builds were no longer getting debug information after #14381.

<!-- 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/14413)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this pull request Dec 1, 2016
@bors-servo
Auto merge of #14413 - servo:jdm-patch-1, r=mbrubeck
Loading status checks…
3576178 
Remove misleading section from non-toplevel Cargo.toml.

This made me waste some time trying to figure out why my builds were no longer getting debug information after #14381.

<!-- 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/14413)
<!-- Reviewable:end -->
@heycam heycam mentioned this pull request Dec 4, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SimonSapin SimonSapin

Assignees

@SimonSapin SimonSapin

Labels
S-tests-failed
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
@nox @highfive @SimonSapin @bors-servo @glennw
You can’t perform that action at this time.