Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upDon't put Cargo into the rustc workspace #40297
Conversation
rust-highfive
assigned
nikomatsakis
Mar 6, 2017
This comment has been minimized.
This comment has been minimized.
|
r? @brson |
rust-highfive
assigned
brson
and unassigned
nikomatsakis
Mar 6, 2017
This comment has been minimized.
This comment has been minimized.
|
(rust_highfive has picked a reviewer for you, use r? to override) |
This was referenced Mar 6, 2017
This comment has been minimized.
This comment has been minimized.
|
This likes like it's putting a submodule into src/vendor, which does not seem like a suitable place for submodules. Why src/vendor? Isn't this where the vendored sources live? |
This comment has been minimized.
This comment has been minimized.
|
Ah yeah unfortunately I could send a commit to Cargo with |
This comment has been minimized.
This comment has been minimized.
|
If the options are putting this in src/vendor or giving cargo its own workspace I prefer the latter. We wouldn't put all the other submodules into src/vendor, or the other tools, I think - istm src/vendor is purely for incidental dependencies, not for the actual tools that are part of Rust. |
This comment has been minimized.
This comment has been minimized.
|
Unfortunately I can't give Cargo it's own workspace due to CI errors caused by existing bugs in Cargo. That to me means the only solution is to put Cargo in some external directory for now. I hope to fix rust-lang/cargo#3192 soon though which I believe should allow us to get the "ideal" hierarchy in the future. @brson how do you feel about |
alexcrichton
force-pushed the
alexcrichton:fix-submodules
branch
from
44d84f3
to
a40e673
Mar 7, 2017
brson
reviewed
Mar 7, 2017
| @@ -559,7 +559,7 @@ pub fn build_rules<'a>(build: &'a Build) -> Rules { | |||
| rules.build("tool-qemu-test-client", "src/tools/qemu-test-client") | |||
| .dep(|s| s.name("libstd")) | |||
| .run(move |s| compile::tool(build, s.stage, s.target, "qemu-test-client")); | |||
| rules.build("tool-cargo", "src/tools/cargo") | |||
| rules.build("tool-cargo", "vendor/cargo") | |||
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
r=me with comment addressed |
alexcrichton
force-pushed the
alexcrichton:fix-submodules
branch
from
a40e673
to
b75116d
Mar 7, 2017
This comment has been minimized.
This comment has been minimized.
|
@bors: r=brson |
This comment has been minimized.
This comment has been minimized.
|
|
This comment has been minimized.
This comment has been minimized.
|
For posterity, we decided to put cargo into a top-level dir for now. That's not scalable but we'll fix the bug in Cargo soon which prevents Cargo from going into a "proper" location. |
This comment has been minimized.
This comment has been minimized.
|
|
This comment has been minimized.
This comment has been minimized.
|
@bors: retry |
frewsxcv
added a commit
to frewsxcv/rust
that referenced
this pull request
Mar 9, 2017
This comment has been minimized.
This comment has been minimized.
|
@bors: p=1 We're not getting nightlies because of this |
This comment has been minimized.
This comment has been minimized.
|
|
arielb1
pushed a commit
to arielb1/rust
that referenced
this pull request
Mar 10, 2017
arielb1
pushed a commit
to arielb1/rust
that referenced
this pull request
Mar 10, 2017
This comment has been minimized.
This comment has been minimized.
|
@bors: retry |
alexcrichton
added a commit
to alexcrichton/rust
that referenced
this pull request
Mar 10, 2017
bors
added a commit
that referenced
this pull request
Mar 10, 2017
arielb1
pushed a commit
to arielb1/rust
that referenced
this pull request
Mar 10, 2017
bors
added a commit
that referenced
this pull request
Mar 10, 2017
arielb1
pushed a commit
to arielb1/rust
that referenced
this pull request
Mar 10, 2017
bors
added a commit
that referenced
this pull request
Mar 10, 2017
bors
added a commit
that referenced
this pull request
Mar 10, 2017
alexcrichton
force-pushed the
alexcrichton:fix-submodules
branch
from
7688222
to
c65996e
Mar 10, 2017
This comment has been minimized.
This comment has been minimized.
|
@bors: r=brson |
This comment has been minimized.
This comment has been minimized.
|
|
alexcrichton
added a commit
to alexcrichton/rust
that referenced
this pull request
Mar 10, 2017
bors
added a commit
that referenced
this pull request
Mar 10, 2017
alexcrichton
added a commit
to alexcrichton/rust
that referenced
this pull request
Mar 11, 2017
bors
added a commit
that referenced
this pull request
Mar 11, 2017
bors
added a commit
that referenced
this pull request
Mar 11, 2017
This comment has been minimized.
This comment has been minimized.
bors
added a commit
that referenced
this pull request
Mar 11, 2017
This comment has been minimized.
This comment has been minimized.
|
@bors: retry |
bors
added a commit
that referenced
this pull request
Mar 11, 2017
This comment has been minimized.
This comment has been minimized.
bors
added a commit
that referenced
this pull request
Mar 11, 2017
This comment has been minimized.
This comment has been minimized.
|
@bors: retry |
alexcrichton commentedMar 6, 2017
•
edited
This causes problems when first cloning and bootstrapping the repository
unfortunately, so let's ensure that Cargo sticks around in its own workspace.
Because Cargo is a submodule it's not available by default on the inital clone
of the rust-lang/rust repository. Normally it's the responsibility of the
rustbuild to take care of this, but unfortunately to build rustbuild itself we
need to resolve the workspace conflicts.
To deal with this we'll just have to ensure that all submodules are in their own
workspace, which sort of makes sense anyway as updates to dependencies as
bugfixes to Cargo should go to rust-lang/cargo instead of rust-lang/rust. In any
case this commit removes Cargo from the global workspace which should resolve
the issues that we've been seeing.
To actually perform this the
cargosubmodule has been moved to a newvendordirectory to ensure it's outside the scope of
src/Cargo.tomlas a workspace.Closes #40284