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

Don't inject clippy into the rls anymore #50767

Merged
merged 1 commit into from
May 15, 2018
Merged

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented May 15, 2018

r? @kennytm

sorry about breaking nightlies.

The issue is that the [patch.crates-io] doesn't work if the versions differ. So every time we update clippy, we can only update it to the verison that rls is depending on.

I'm disabling the injection of clippy into rls for now. I'm not sure how to do this properly. We could

  • add a version check, so rls only builds clippy if its dependency clippy is the same as the submodule clippy
  • do something crazy like auto-patching the Cargo.toml of the rls tool repo to just use a path dependency on clippy
  • build crates-io clippy instead of submodule clippy and gate clippy injection on that
    • that's somewhat automatic, and is essentially what is necessary right now, but done manually
  • make clippy 0.1.* instead of 0.0.* and update patch versions for nightly updates and minor version updates for clippy_lints api changes.
    • not sure how semver-great this is

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 15, 2018
@kennytm
Copy link
Member

kennytm commented May 15, 2018

cc @nrc (#48097).

@kennytm kennytm added the T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. label May 15, 2018
@kennytm
Copy link
Member

kennytm commented May 15, 2018

@bors r+ p=5

Restore nightlies hopefully.

@bors
Copy link
Contributor

bors commented May 15, 2018

📌 Commit 0370c38 has been approved by kennytm

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 15, 2018
kennytm added a commit to kennytm/rust that referenced this pull request May 15, 2018
Don't inject clippy into the rls anymore

r? @kennytm

sorry about breaking nightlies.

The issue is that the `[patch.crates-io]` doesn't work if the versions differ. So every time we update clippy, we can only update it to the verison that rls is depending on.

I'm disabling the injection of clippy into rls for now. I'm not sure how to do this properly. We could

* add a version check, so rls only builds clippy if its dependency clippy is the same as the submodule clippy
* do something crazy like auto-patching the Cargo.toml of the rls tool repo to just use a path dependency on clippy
* build crates-io clippy instead of submodule clippy and gate clippy injection on that
    * that's somewhat automatic, and is essentially what is necessary right now, but done manually
* make clippy 0.1.* instead of 0.0.* and update patch versions for nightly updates and minor version updates for `clippy_lints` api changes.
    * not sure how semver-great this is
@bors
Copy link
Contributor

bors commented May 15, 2018

⌛ Testing commit 0370c38 with merge f0fdaba...

bors added a commit that referenced this pull request May 15, 2018
Don't inject clippy into the rls anymore

r? @kennytm

sorry about breaking nightlies.

The issue is that the `[patch.crates-io]` doesn't work if the versions differ. So every time we update clippy, we can only update it to the verison that rls is depending on.

I'm disabling the injection of clippy into rls for now. I'm not sure how to do this properly. We could

* add a version check, so rls only builds clippy if its dependency clippy is the same as the submodule clippy
* do something crazy like auto-patching the Cargo.toml of the rls tool repo to just use a path dependency on clippy
* build crates-io clippy instead of submodule clippy and gate clippy injection on that
    * that's somewhat automatic, and is essentially what is necessary right now, but done manually
* make clippy 0.1.* instead of 0.0.* and update patch versions for nightly updates and minor version updates for `clippy_lints` api changes.
    * not sure how semver-great this is
@bors
Copy link
Contributor

bors commented May 15, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: kennytm
Pushing f0fdaba to master...

@bors bors merged commit 0370c38 into rust-lang:master May 15, 2018
@rust-highfive
Copy link
Collaborator

📣 Toolstate changed by #50767!

Tested on commit f0fdaba.
Direct link to PR: #50767

🎉 rls on windows: build-fail → test-fail.
🎉 rls on linux: build-fail → test-fail.

rust-highfive added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request May 15, 2018
Tested on commit rust-lang/rust@f0fdaba.
Direct link to PR: <rust-lang/rust#50767>

🎉 rls on windows: build-fail → test-fail.
🎉 rls on linux: build-fail → test-fail.
@oli-obk oli-obk deleted the rls_clippy branch June 15, 2020 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants