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

Start using subtrees for external dependencies #26042

Closed
wants to merge 9 commits into from

Conversation

alexcrichton
Copy link
Member

This commit removes the in-tree copies of lib{term,rustc_serialize,getopts} in favor of the upstream versions in the rust-lang github organization. The purpose of this is to both reduce duplication of code as well as enabling easier addition of external dependencies into the build of the compiler itself. There are a few points of note here:

  • All external crates have been moved into a new src/external folder. This folder has a README explaining the git subtree setup as well as how to add new crates and update existing ones.
  • External crates are still compiled as unstable (e.g. not accessible from stable code) via the #![staged_api] attribute guarded by the rust_build cfg.
  • The API of getopts changed quite radically from the in-tree version, and all consumers were updated accordingly
  • The name serialize has been purged as all crates now just explicitly link to rustc_serialize.

cc the initial discussion thread about this topic.

git-subtree-dir: src/external/term
git-subtree-split: 6822aed1980f572f5d241c7bacf6971b5e2775ed
git-subtree-dir: src/external/rustc_serialize
git-subtree-split: e3115f387f98561a38caa147b776f01b4d4bb087
git-subtree-dir: src/external/getopts
git-subtree-split: 478ce9bdb98096afa066e79c64efb6683a8fa112
cba058f Add decoding into boxed slices

git-subtree-dir: src/external/rustc_serialize
git-subtree-split: cba058fb8bd106d9dcdef773335a633ca4aaa2ce
This commit updates our own makefiles to take into account external crates in
the `src/external` folder now. Additionally it updates all current consumers of
the crates that have transitioned to their external sources to the new APIs that
are upstream.

This commit also adds a README to the `src/external` folder explaining the
management strategy for those libraries.
@rust-highfive
Copy link
Collaborator

r? @huonw

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member Author

r? @brson

cc @rust-lang/libs

@rust-highfive rust-highfive assigned brson and unassigned huonw Jun 5, 2015
@brson
Copy link
Contributor

brson commented Jun 5, 2015

I want to think about this pretty hard if we're going to adopt sub_trees_ in addition to our sub_modules_. We've already got a lot of experience with submodules. Adding yet another thing for maintainers to understand is a big step.

@retep998
Copy link
Member

retep998 commented Jun 6, 2015

The term that you are bringing in depends on winapi on Windows. I don't see a subtree for winapi though. I was just wondering how you intend to resolve that.

@ahmedcharles
Copy link
Contributor

I agree with @brson about using both submodules and subtrees, even if development isn't supposed to happen in the rust tree itself.

@alexcrichton
Copy link
Member Author

@brson, do you have some specific concerns you're worried about? From what I understand, the benefits are:

  • It doesn't matter if the remote repositories change locations or go away.
  • No extra management is necessary on those just building Rust. You only need to touch the subtree if you're updating it or adding a new one.
  • No extra step is needed to check out the subtree

The downsides being

  • The subtree can be modified to be different than the upstream copy.
  • It looks like the subtree can be modified without worrying about the upstream copy.

I feel that the benefit of not having to worry about the remote repository going away far outweighs the downsides, personally.


@retep998

I forgot about that.

@bors
Copy link
Contributor

bors commented Jun 8, 2015

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

@alexcrichton
Copy link
Member Author

Hm ok, we don't have a great way to deal with platform-specific dependencies right now as part of the build system, so moving to an out-of-tree term will likely not happen just yet. I'm also losing a bit of steam for this change as the future prospects don't seem super great, so for now I'm going to close this.

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.

7 participants