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

Potential stable point release tools issues #56726

Closed
nrc opened this issue Dec 11, 2018 · 10 comments
Closed

Potential stable point release tools issues #56726

nrc opened this issue Dec 11, 2018 · 10 comments
Labels
stable-accepted Accepted for backporting to the compiler in the stable channel. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue.

Comments

@nrc
Copy link
Member

nrc commented Dec 11, 2018

The first is probably not worth making a point release just for that. The second is probably worth a point release, but doesn't have a fix yet. @Xanewok is investigating.

The other major tools issues are with rustup which won't need a point release.

cc @rust-lang/release

@nrc nrc added T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. stable-nominated Nominated for backporting to the compiler in the stable channel. labels Dec 11, 2018
@Xanewok
Copy link
Member

Xanewok commented Dec 11, 2018

Just the go-to def fix is at https://github.com/Xanewok/rust/commits/1.31-stable-gotodef (one commit ahead of current rust/stable branch), confirmed locally that it works. (RLS is checked out with a branch that points to patched rls-analysis: https://github.com/rust-dev-tools/rls-analysis/tree/1.31-stable-src-prefix-fix)

@Xanewok
Copy link
Member

Xanewok commented Dec 13, 2018

This pulls a specific commit from rust-dev-tools/rls-analysis as opposed to the one from crates.io (Xanewok@c3277af#diff-5d9bf57935c52008319cccc88d4d737dR1813). Will that be a problem?

@pietroalbini
Copy link
Member

cc @rust-lang/dev-tools, should this be stable-accepted?

Also, I'm not too familiar with RLS, what would be the steps to backport the changes to stable?

@Xanewok
Copy link
Member

Xanewok commented Dec 16, 2018

For the 2nd point (go to definition) we'd have to merge Xanewok@c3277af (updates rls-analysis to branched version in Rust lockfile and checks out branched RLS using the patched rls-analysis).
The other approach would be to [patch] Cargo.toml to use the patched rls-analysis version and update it in Cargo.toml, although iirc @nrc was against that.

I think I'm confused as to what exactly version is on stable due to edition (do stable and beta point to the same release with nightly being only couple of days younger now?), so I'm not exactly sure what and how we should backport changes also including the fix for hover infinite loop regression.

@Mark-Simulacrum Mark-Simulacrum added the stable-accepted Accepted for backporting to the compiler in the stable channel. label Dec 16, 2018
@nrc
Copy link
Member Author

nrc commented Dec 17, 2018

I've published the rls-analysis branch as 0.16.10-stable. I created the RLS branch stable-update-1.31.7. I have a PR for Rust in progress, but didn't quite finish it up tonight.

Thanks @Xanewok for doing all the real work!

@pietroalbini
Copy link
Member

@nrc don't worry about the rust PR, I'll just include the changes in the stable one.

@pietroalbini
Copy link
Member

Also cherry-picked rust-lang/rls#1170 inside the stable-update-1.31.7 branch.

@pietroalbini pietroalbini removed the stable-nominated Nominated for backporting to the compiler in the stable channel. label Dec 17, 2018
@pietroalbini
Copy link
Member

Uh, are those issues fixed on beta?

@Xanewok
Copy link
Member

Xanewok commented Jan 2, 2019

It doesn't seem like it;

rust-lang/rls@77ff073 is the last commit on beta, while the infinite hover fix (rust-lang/rls@bfa1371) is just one PR further. In this case, could we just bump RLS a few commits ahead on beta channel?

However I think goto-def should work, since I believe the src/ path change actually got into beta in time. I'll check the beta channel if this problem persists.
EDIT: Yep, gotodef works, so the only thing that remains is to fast-forward the RLS by 2 commits on beta and we're good to go 🎉

Xanewok added a commit to Xanewok/rust that referenced this issue Jan 3, 2019
Beta backport of a fix that already was backported to stable,
see rust-lang#56726 and
rust-lang/rls#1170 for the underlying RLS issue.

Also includes the fix for rust-lang/rls#1154
(respecting target-dir specified in .cargo/config for RLS artifacts).
@nrc
Copy link
Member Author

nrc commented Jan 7, 2019

Uh, are those issues fixed on beta?

Ah, curses I was planning on updating beta to most recent RLS at some point before the uplift, but just uplifting that fix is fine too. Thanks for doing it @Xanewok !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stable-accepted Accepted for backporting to the compiler in the stable channel. T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants