Documents managing tools in the Rust repo #27
Conversation
tools-in-rust-repo.md
Outdated
| [stability docs](https://github.com/nrc/dev-tools-team/blob/master/stability/README.md). | ||
|
|
||
| When adding tools or making significant changes, please be sure to cc @nrc and | ||
| @aclexcrichton on PRs. Adding tools to the Rust repo requires approval from the |
| repository. For information on requirements for such tools see the | ||
| [stability docs](https://github.com/nrc/dev-tools-team/blob/master/stability/README.md). | ||
|
|
||
| When adding tools or making significant changes, please be sure to cc @nrc and |
There was a problem hiding this comment.
Also fine being pinged on all Clippy changes
tools-in-rust-repo.md
Outdated
| is updated. Changes to `Cargo.lock` must be committed along with `Cargo.toml` | ||
| changes. | ||
|
|
||
| To run tests for a tool, use `./x.py check src/tools/rls`. Note that it is |
| their tool continues to build and pass tests in the Rust repo. For non-trivial | ||
| fixes, tool devs should be happy to help fix breaking changes. | ||
|
|
||
| How to land a breaking change: |
There was a problem hiding this comment.
I was actually assuming that the default would be "stop building the tool, wait for a nightly, fix the tool, re-enable the tool" in the sense that this process is pretty onerous. The other downside of this approach is that it's adding burden to compiler devs to fix tools, where we aren't necessarily willing to shoulder that burden on compiler devs for all commits.
Do you think this'll be more common than the "exceptions" branch though?
There was a problem hiding this comment.
When we discussed this in the past we had the 'exception' process as what we wanted to do, with some infra in the build system to make this easy to do, to prevent doing it on beta or in the last week of a cycle, and to require a visible opt-in. I still think that is the better long term solution. We decided we should do the 'non-exception' process for the time-being until we add code for the 'exception' process, despite the onerous-ness. I still think that is a good plan. I think encouraging the 'exception' process without the code to help is a bit risky.
There was a problem hiding this comment.
But fundamentally this isn't a solution, right? We're forcing all tools included to have broken CI to land changes?
I guess I'm just surprised at this because this is a fundamental problem we can't fix, and I'd be much more opposed to including projects like clippy if we require all developers to fix clippy rather than just those knowledgeable.
It's of course painful to take the "exceptions" route but isn't that the pain of nightly?
There was a problem hiding this comment.
Sorry, I didn't see this comment before merging.
The tool CI never breaks on master, only on a branch. So, I think it depends on the breaking change - so far with the RLS, the changes have been trivial to fix (e.g., adding () to every use of span.lo/hi) and the compiler changer has made the change, and I've done the branch dance on the RLS-side (e.g., rust-lang/rust#43968). I imagine that if the change needed more work in the tool, then the tool authors would do the work, but it would be basically the same process (I've tried to stress that it is the tool authors responsibility here).
The trouble with the exceptions route is that it leads to a tool not being built, which means that rustup will look for it and not find it, and presumably error out. We really need infrastructure support before I'm happy recommending that route. (And in practice the non-exception route is working OK, and with the incremental path to enabling tools, we should have time to observe if it is continuing to work OK).
There was a problem hiding this comment.
I guess? At some point I'm not personally responsible for the tools at hand, so I'll let y'all call the shots...
I really am worried about the contributor experience, though. I'd dread wanting to make a change to rustc only realizing it breaks 3 tools so I have to go and update 3 submodules, push to new branches, coordinate with maintainers, etc. If this starts happening a lot it seems like it can become a real drag on iterating in the compiler?
|
One example of "how to preserve commits forever" may want to link to the LLVM/compiler-rt repository which has a ton of random branches for this reason. |
|
This doesn't currently spell out how to get a tool into rustup and distributed there, but do you think we have enough in our head to do that right now? I'd be fine if that was a follow-up to this! Oh and also, this looks great! Thanks so much @nrc for writing this up! |
Hmm, I'm a bit vague on this actually. I added a line about editing build-manifest, and there was already stuff for creating the tarball, iirc, that is all that needed to be done? I vaguely remember something about install.sh, but I can't find the actual script file anywhere. |
|
Oh for the rustup bits I meant moreso about the name like |
|
OK, cool, I'll come back and add a note about that once the code lands |
cc #15
r? @alexcrichton