Skip to content

fix: Code: use Uri.toString() for URLs #9281

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

Merged
merged 1 commit into from
Jun 15, 2021
Merged

Conversation

wxb1ank
Copy link
Contributor

@wxb1ank wxb1ank commented Jun 15, 2021

I believe this should fix #9280. Testing is needed but I wanted to quickly push a hotfix.

Edit: Forgot to explain what the issue was. From my reply in #9280:

Looks like this is a regression from #8951. Specifically, in downloadFile() from /editors/code/src/net.ts, a vscode.Uri's .path property is mistakenly used as a URL (I think the .toString() method should work?). I don't immediately see any other sites where this is an issue.

Essentially, the .path property really only works for filesystem URIs, so URLs need to use a method that concatenates all URI components (this is what .toString() does). You can read about this here.

@wxb1ank wxb1ank changed the title Use Uri.toString() for URLs fix: Use Uri.toString() for URLs in VS Code extension Jun 15, 2021
@wxb1ank wxb1ank changed the title fix: Use Uri.toString() for URLs in VS Code extension fix: Code: use Uri.toString() for URLs Jun 15, 2021
@lnicola
Copy link
Member

lnicola commented Jun 15, 2021

This can't hurt, right?

bors r+

@bors
Copy link
Contributor

bors bot commented Jun 15, 2021

@bors bors bot merged commit bd87882 into rust-lang:master Jun 15, 2021
@matklad
Copy link
Member

matklad commented Jun 15, 2021

We probably want to kick a nightly release

@matklad
Copy link
Member

matklad commented Jun 15, 2021

Did a release, also opened #9283.

Thanks @wxb1ank for lightning-fast fix by the way!

@wxb1ank
Copy link
Contributor Author

wxb1ank commented Jun 15, 2021

Did a release, also opened #9283.

Thanks @wxb1ank for lightning-fast fix by the way!

No problem, sorry for the regression in the first place. I should've tested update functionality in #8951. Oh well :)

@SomeoneToIgnore
Copy link
Contributor

@wxb1ank , maybe that helps you:

When changing the client, I was able to manually debug the functionality with the "Run Installed Extension" (see the screenshot).

To fully emulate various cases (weekly release tries to bootstrap, nightly release tries to bootstrap) you'll have to amend the package.json and put the proper version, releaseTag values and remove $generated-start, $generated-end ones.

Simplest way to "properly" adjust the package.json would be to download the corresponding rust-analyzer.vsix plugin from the r-a releases page, open it with any unarchiver, find the package.json there and copy-paste the contents over the one in the repo.
Then you start the debug and the plugin will pick up the amended package.json automatically, considering itself a weekly/nightly released plugin.

That should be enough to debug the bootstrap functionality on one platform of yours. I suggest testing at least two: Windows and Linux/Mac since the path handling is different in those which can lead to subtle bugs.

image

@wxb1ank
Copy link
Contributor Author

wxb1ank commented Jun 15, 2021

@SomeoneToIgnore Thank you!! That makes testing much easier :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bootstrap error [TypeError: Only absolute URLs are supported
4 participants