Skip to content

Rls and Rustfmt stuff #44769

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
wants to merge 4 commits into from
Closed

Rls and Rustfmt stuff #44769

wants to merge 4 commits into from

Conversation

nrc
Copy link
Member

@nrc nrc commented Sep 22, 2017

Updates the RLS and Rustfmt, uses the toolstate stuff for allowing disabling rls and rustfmt, and renames the rls component to rls-preview on nightly.

r? @alexcrichton

work.join(&format!("{}-{}", pkgname(build, "rls"), target)).join("rls-preview")
};
cp_r(&rls_path, &exe.join("rls"));
cp_r(&work.join(&format!("{}-{}", pkgname(build, "rls"), target)).join("rls"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be .join("rls-preview")? (to stay the same as non-nightly)

@alexcrichton
Copy link
Member

Looks good! Just one minor comment

I do think, though, that the toolstate.toml isn't going to be enough here. If we switch the rls to "not building" then I think most of ./x.py dist will fail with weird errors, meaning we can't actually land a commit into master where the RLS isn't building yet.

Fixing this I think will require some rather invasive changes. I started this locally a few days ago but haven't had a chance to finish it :(. The hardest part is going to be modifying the combined installers, notably the pkg/exe/msi installers as their built-in manifests are going to change based on whether the RLS is available or not.

That all being said, I think this is fine to land in the meantime. We'll need to keep the rls/rustfmt building to actually land any changes, but that's just the status quo.

@nrc
Copy link
Member Author

nrc commented Sep 22, 2017

Comment addressed.

Hmm, yeah, keeping the installers going is going to be a bit tricky.

@alexcrichton
Copy link
Member

@bors: r+

@bors
Copy link
Collaborator

bors commented Sep 22, 2017

📌 Commit 26d6a2f has been approved by alexcrichton

@shepmaster shepmaster added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Sep 22, 2017
@shepmaster
Copy link
Member

[00:56:03] failures:
[00:56:03]     [run-make] run-make/pretty-expanded-hygiene
[00:56:03]     [run-make] run-make/pretty-print-path-suffix

Maybe from error: multiple input filenames provided ?

@bors
Copy link
Collaborator

bors commented Sep 24, 2017

⌛ Testing commit 26d6a2f9bc97d43f0d5bd2ba98d1431e23bf670a with merge 7a0f64bdae8cb274a9ba91d20615a912563dcf87...

@bors
Copy link
Collaborator

bors commented Sep 24, 2017

💔 Test failed - status-travis

@nrc
Copy link
Member Author

nrc commented Sep 24, 2017

Same error on Homu, but probably due to #44785 (comment)

@carols10cents
Copy link
Member

travis is still failing :(

@carols10cents carols10cents added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 25, 2017
@bors
Copy link
Collaborator

bors commented Sep 27, 2017

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

@alexcrichton
Copy link
Member

I've folded this into #44785

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants