Skip to content

ship rust analyzer - #72978

Merged
bors merged 1 commit into
rust-lang:masterfrom
matklad:ship-rust-analyzer
Jul 4, 2020
Merged

ship rust analyzer#72978
bors merged 1 commit into
rust-lang:masterfrom
matklad:ship-rust-analyzer

Conversation

@matklad

@matkladmatklad commented Jun 4, 2020•
edited by Mark-Simulacrum
Loading

Copy link
Copy Markdown
Contributor

This successfully builds rust-analyzer as a part of rust repo. I haven't yet added required changes to dist.rs -- seems like I just have to copy-paste quite a bit of code I don't really understand :-)

@rust-highfive

rust-highfive commented Jun 4, 2020

Copy link
Copy Markdown
Contributor

⚠️Warning⚠️

  • These commits modify submodules.

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 4, 2020
@Mark-Simulacrum

Mark-Simulacrum commented Jun 4, 2020

Copy link
Copy Markdown
Member

Yeah, the dist.rs modification strategy is like that for everyone I think :)

But this looks like a good start to me -- let me know if you have specific questions that I can help answer; once we think dist.rs is in good shape a try build should produce sufficient artifacts for testing on linux (x86_64) and once that's working we can land it! 🎉

@matklad
matkladforce-pushed the ship-rust-analyzer branch 4 times, most recently from 4b651c7 to 8003ab3CompareJune 4, 2020 14:14
@matkladmatklad changed the title WIP: ship rust analyzership rust analyzerJun 4, 2020
matklad
matklad commented Jun 4, 2020
View reviewed changes
Comment threadsrc/bootstrap/dist.rs Outdated
@matklad

matklad commented Jun 4, 2020

Copy link
Copy Markdown
ContributorAuthor

Ready for review @Mark-Simulacrum I think.

@matklad
matkladforce-pushed the ship-rust-analyzer branch from 8003ab3 to 22c9db1CompareJune 4, 2020 14:28
Mark-Simulacrum
Mark-Simulacrum reviewed Jun 4, 2020
View reviewed changes
Comment threadsrc/bootstrap/dist.rs Outdated
Comment threadsrc/bootstrap/dist.rs Outdated
Comment threadsrc/bootstrap/dist.rs Outdated
Comment threadsrc/bootstrap/install.rs Outdated
@Mark-Simulacrum

Mark-Simulacrum commented Jun 4, 2020

Copy link
Copy Markdown
Member

All of those shouldn't affect a try build I think so let's see if this is doing what it's supposed to, @bors try

@bors

bors commented Jun 4, 2020

Copy link
Copy Markdown
Collaborator

⌛ Trying commit 22c9db138d7dda91c127c241919acc50a5921dcf with merge 37d6cce42752fb1b3ae7414d5c142c99dffc53f5...

@matklad
matkladforce-pushed the ship-rust-analyzer branch from 22c9db1 to 7e615eaCompareJune 4, 2020 14:48
matklad
matklad commented Jun 4, 2020
View reviewed changes
Comment threadsrc/bootstrap/install.rs Outdated
@matklad
matkladforce-pushed the ship-rust-analyzer branch from 7e615ea to 8cbd785CompareJune 4, 2020 14:53
@ehuss

ehuss commented Jun 4, 2020

Copy link
Copy Markdown
Contributor

Will this be tracked in toolstate in the future?

@Mark-Simulacrum

Mark-Simulacrum commented Jun 4, 2020

Copy link
Copy Markdown
Member

@ehuss I don't think so -- rust-analyzer builds with a stable compiler today so unless that changes in the future (and I think we don't want it to) it shouldn't break on changes to the compiler so there's no reason for toolstate really.

@Mark-Simulacrum

Mark-Simulacrum commented Jun 5, 2020

Copy link
Copy Markdown
Member

Heh, looks like rust-analyzer failed to build on our dist builders. I'm not sure whether this is "expected" -- I guess maybe we just need to install inotify? There's also the lutimes warning which is somewhat concerning.

It does look like force pushing kills bors from notifying that the try build is done (but the try build still completes).

"-Wl,-rpath,$ORIGIN/../lib"�[0m
2020-06-04T17:18:41.4210399Z �[0m �[0m�[0m�[1m�[38;5;12m= �[0m�[0m�[1mnote�[0m�[0m: /checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/release/deps/libfiletime-cd4b67c5479ae270.rlib(filetime-cd4b67c5479ae270.filetime.4z3loc0p-cgu.4.rcgu.o): In function `filetime::imp::utimes::set_times::hb28e25c679b44a1a':�[0m
2020-06-04T17:18:41.4211763Z �[0m filetime.4z3loc0p-cgu.4:(.text._ZN8filetime3imp6utimes9set_times17hb28e25c679b44a1aE+0x287): warning: warning: lutimes is not implemented and will always fail�[0m
2020-06-04T17:18:41.4215999Z �[0m /checkout/obj/build/x86_64-unknown-linux-gnu/stage1-tools/x86_64-unknown-linux-gnu/release/deps/libinotify-cd7a29090309d6ed.rlib(inotify-cd7a29090309d6ed.inotify.bxraksv4-cgu.3.rcgu.o): In function `inotify::inotify::Inotify::init::he432cafdcf6fb733':�[0m
2020-06-04T17:18:41.4217235Z �[0m inotify.bxraksv4-cgu.3:(.text._ZN7inotify7inotify7Inotify4init17he432cafdcf6fb733E+0x10): undefined reference to `inotify_init1'�[0m
2020-06-04T17:18:41.4218077Z �[0m clang-9: error: linker command failed with exit code 1 (use -v to see invocation)�[0m
2020-06-04T17:18:41.4218761Z �[0m �[0m
2020-06-04T17:18:41.4218902Z 2020-06-04T17:18:41.4243114Z �[0m�[1m�[38;5;9merror�[0m�[0m�[1m: aborting due to previous error�[0m
2020-06-04T17:18:41.4243571Z 2020-06-04T17:18:41.4450968Z �[0m�[0m�[1m�[31merror�[0m�[1m:�[0m could not compile `rust-analyzer`.

@mati865

mati865 commented Jun 5, 2020

Copy link
Copy Markdown
Member

@Mark-Simulacrum you cannot just install inotify.
inotify_init1 comes from GLIBC (2.9 should be fine?) but Linux dist uses GLIBC 2.5 for compatibility reasons.

@matklad

matklad commented Jun 5, 2020

Copy link
Copy Markdown
ContributorAuthor

Yup, as mati865 said, it seems like our file watching library requires glibc 2.9. I don't think it's reasonable to bump glibc version we use to build the rust toolchain, so we'd have to change something on rust-analyzer side. This is somewhat sad, because our file-watching functionality is known to be broken in various ways anyway (by default, we use editor-side file watching if that's available).

@matkladmatklad mentioned this pull request Jun 5, 2020
Closed
7 tasks
@lnicola

lnicola commented Jun 5, 2020•
edited
Loading

Copy link
Copy Markdown
Member

CC #62516. I think we'd be able to drop support for CentOS 5 in December at earliest.

@matkladmatklad added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 5, 2020
@matklad

matklad commented Jun 5, 2020

Copy link
Copy Markdown
ContributorAuthor

blocked on resolving the issue in Rust inotify wrappar: rust-lang/rust-analyzer#4224 (comment).

@matklad
matkladforce-pushed the ship-rust-analyzer branch from 8cbd785 to 326888aCompareJune 5, 2020 14:32
@bors

bors commented Jul 3, 2020

Copy link
Copy Markdown
Collaborator

🌲 The tree is currently closed for pull requests below priority 100, this pull request will be tested once the tree is reopened

@borsbors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 3, 2020
@bors

bors commented Jul 3, 2020

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 92b5dd76bc30e985cfc989438838e7d9cda16b93 with merge fcdd79ab164987b04c0e6a4a1e0adcaf6040cb2c...

@bors

bors commented Jul 3, 2020

Copy link
Copy Markdown
Collaborator

💔 Test failed - checks-actions

@borsbors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 3, 2020
@matklad

matklad commented Jul 3, 2020

Copy link
Copy Markdown
ContributorAuthor

So, this failed on power because we use AtomicU64 in salsa. Let me check if this is fixable in salsa...

@matklad
Add rust-analyzer submodule
058c1b6
The current plan is that submodule tracks the `release` branch of
rust-analyzer, which is updated once a week.
rust-analyzer is a workspace (with a virtual manifest), the actual
binary is provide by `crates/rust-analyzer` package.
Note that we intentionally don't add rust-analyzer to `Kind::Test`,
for two reasons.
*First*, at the moment rust-analyzer's test suite does a couple of
things which might not work in the context of rust repository. For
example, it shells out directly to `rustup` and `rustfmt`. So, making
this work requires non-trivial efforts.
*Second*, it seems unlikely that running tests in rust-lang/rust repo
would provide any additional guarantees. rust-analyzer builds with
stable and does not depend on the specifics of the compiler, so
changes to compiler can't break ra, unless they break stability
guarantee. Additionally, rust-analyzer itself is gated on bors, so we
are pretty confident that test suite passes.
@matklad
matkladforce-pushed the ship-rust-analyzer branch from 92b5dd7 to 058c1b6CompareJuly 3, 2020 14:55
@matklad

matklad commented Jul 3, 2020

Copy link
Copy Markdown
ContributorAuthor

Updated salsa and checked that rust-analyzer cargo checks locally with powerpc target after update

@bors r=Mark-Simulacrum

@bors

bors commented Jul 3, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit 058c1b6 has been approved by Mark-Simulacrum

@borsbors 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 Jul 3, 2020
@bors

bors commented Jul 3, 2020

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 058c1b6 with merge 743a9fab5f5e724bf0b5364e802f3d1f8673e079...

@bors

bors commented Jul 3, 2020

Copy link
Copy Markdown
Collaborator

💔 Test failed - checks-actions

@borsbors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 3, 2020
@matklad

matklad commented Jul 3, 2020

Copy link
Copy Markdown
ContributorAuthor

Some cargo test failed. Seems unrelated?

@Manishearth

Manishearth commented Jul 3, 2020

Copy link
Copy Markdown
Member

@bors retry

seems like a network issue

@borsbors 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 Jul 3, 2020
@bors

bors commented Jul 4, 2020

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 058c1b6 with merge 0cd7ff7...

@bors

bors commented Jul 4, 2020

Copy link
Copy Markdown
Collaborator

☀️ Test successful - checks-actions, checks-azure
Approved by: Mark-Simulacrum
Pushing 0cd7ff7 to master...

@borsbors added the merged-by-bors This PR was explicitly merged by bors. label Jul 4, 2020
@bors
bors merged commit 0cd7ff7 into rust-lang:masterJul 4, 2020
@borsbors mentioned this pull request Jul 4, 2020
Merged
6 tasks
@matklad
matklad deleted the ship-rust-analyzer branch July 4, 2020 13:11
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Mark-SimulacrumMark-SimulacrumMark-Simulacrum left review comments
@bjorn3bjorn3bjorn3 left review comments
+1 more reviewer
@pickfirepickfirepickfire left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@Mark-SimulacrumMark-Simulacrum

Labels

merged-by-borsThis PR was explicitly merged by bors.S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

14 participants

@matklad@rust-highfive@Mark-Simulacrum@bors@ehuss@mati865@lnicola@rust-timer@cuviper@Dylan-DPC-zz@JohnCSimon@Manishearth@pickfire@bjorn3