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

Use git subtree (instead of git submodule) for external dependencies #266

Closed
oli-obk opened this issue Apr 1, 2020 · 5 comments
Closed
Labels
major-change A proposal to make a major change to rustc major-change-accepted A major change proposal that was accepted

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Apr 1, 2020

TL;DR

Replace all our git submodules with git subtrees. This allows us to gate CI on tools, since contributors can just fix tools in tree and tool devs synchronize the in-tree state with their repo at their leisure.

Links and Details

Git submodules shallowly add just the commit id of the remote repository to the rustc repository. This means contributors need to use git submodule update whenever they change branches. It also means, that if you make a change to rustc that breaks a submodule, you need to do a second PR just for the submodule and point the submodule at that PR. This can quickly get out of hand with quickly changing projects like rustc and its tools.

Git subtrees on the other hand basically work like a monorepo. All tool sources are in tree, and git subtree is just a synchronization tool to push changes that happened to the in-tree tool sources back to the tool repository and vice versa. Git subtree preserves commits on both sides, so changes to the tool are fully traceable in the rustc repository and rustc changes that touched the tool sources show up in the tool repository.

This creates some synchronization overhead for tool developers, but the current status quo with submodules is worse, because we have tool breakages and synchronization overhead. We don't have much experience with git subtree, so we'll have to see whether this is actually a better experience. The fact that we get rid of tool breakages without slowing down tool development in its own repo seems like a great boon even if the synchronization overhead stays the same.

Mentors or Reviewers

@oli-obk oli-obk added the major-change A proposal to make a major change to rustc label Apr 1, 2020
@Centril
Copy link
Contributor

Centril commented Apr 1, 2020

@Mark-Simulacrum is probably the better technical reviewer here though :)

@nikomatsakis
Copy link
Contributor

I second the overall approach -- I'd like to see experimentation here, and it seems clear that the current tooling isn't cutting it!

I understand the plan to be that we will start with one tool (clippy) and experiment until we're happy, and then expand to other tools?

The main thing I would like us to evaluate is how effective we are at doing the two-way sync. In particular, whose responsibility is it? And how do they know when to do it? (Every so often? Monitoring for relevant PRs? Can they get pinged automatically by a bot when a PR lands that touches one of the tools?) Anyway, I don't see a need to block on that, I just think it's a good thing to pay attention to and try to document.

@Centril
Copy link
Contributor

Centril commented Apr 3, 2020

  • When do we want to consider whether to move the next tool to a subtree (e.g., Miri)? Maybe give it 6 weeks?

@francbetacc

This comment has been minimized.

@nikomatsakis nikomatsakis added final-comment-period The FCP has started, most (if not all) team members are in agreement and removed final-comment-period The FCP has started, most (if not all) team members are in agreement labels Apr 13, 2020
@nikomatsakis
Copy link
Contributor

So @oli-obk announced this in the meeting on 2020-04-02 and I seconded shortly thereafter. We didn't really have the policy all figured out here, but nonetheless I'm going to declare this MCP accepted since I think conversation has been going on for a while and the relevant stakeholders are all aware.

(cc @rust-lang/compiler)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major-change A proposal to make a major change to rustc major-change-accepted A major change proposal that was accepted
Projects
None yet
Development

No branches or pull requests

5 participants