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

Add support for custom root URL path through command line option --route-base-url-path #201764

Closed
wants to merge 7 commits into from

Conversation

oakaigh
Copy link
Contributor

@oakaigh oakaigh commented Jan 4, 2024

Setup (HOWTO Test)

  • Specify a base URL: <base-url-path>
  • Git clone the repo
  • Build VSCode per "How-to-Contribute"
  • Run from CLI
./scripts/code-server.sh --port 9888 --route-base-url-path <base-url-path>
  • Server available at http://localhost:9888/<base-url-path>
  • (Optionally) Run a proxy routing requests to <base-url-path>
  • Profit

Example

  • Run from CLI
./scripts/code-server.sh --port 9888 --route-base-url-path /user/someuser/proxy
  • Server available at http://localhost:9888/user/someuser/proxy

Notes

  • Non-absolute paths should work; but not all the time especially since sources files are then loaded relative to the current URL, which changes as, for instance, user opens a workspace. The server, however, only serves files from a static path and "pushes" that path to the client at loading time. The server is not informed of the expected path change whatsoever.
  • Trailing / does not need to be present in the path.

Related Issues

gitpod-io#320
#153679
#165391

@oakaigh oakaigh changed the title add support for custom root URL path through command line option --route-base-url-path Add support for custom root URL path through command line option --route-base-url-path Jan 4, 2024
@oakaigh
Copy link
Contributor Author

oakaigh commented Jan 4, 2024

@microsoft-github-policy-service agree

@chrmarti chrmarti assigned aeschli and unassigned chrmarti Jan 8, 2024
@aeschli
Copy link
Contributor

aeschli commented Jan 8, 2024

Thanks!

Can you keep the PR as minimal as possible? E.g. don't modify .gitignore or uti.ts.

What's not so nice is the global RemotePaths.

@oakaigh
Copy link
Contributor Author

oakaigh commented Jan 10, 2024

@aeschli

What's not so nice is the global RemotePaths.

Well yes, it’s not pretty. The design choice was kind of borrowed from RemoteAuthoritiesImpl I guess? (RemoteAuthoritiesImpl initiates a global instance RemoteAuthorities, whose properties are set within the constructors of the *Services - this is even uglier imo)
I don’t see any other way of doing it other than changing all classes that use RemotePaths to take a RemotePathsImpl instance instead, which would likely involve a lot of work.

Can you keep the PR as minimal as possible? E.g. don't modify .gitignore or uti.ts.

Sure! Updates coming soon.

@aeschli
Copy link
Contributor

aeschli commented Jan 10, 2024

I tried to avoid the global RemotePaths:
https://github.com/microsoft/vscode/tree/aeschli/serverRootPath

@oakaigh
Copy link
Contributor Author

oakaigh commented Jan 11, 2024

I forgot to mention: the JavaScript Debug Terminal does not seem to be working in this PR right now; this causes the test to fail. I currently do not have time to investigate. Any help is appreciated! (fixed by #202491)

@aeschli Ideally instead of passing the path as a string we could make it a service like PathResolverService to allow for more flexibility. What do you think?
Also it’d be great to get rid of this product: { quality?: string; commit?: string } thing and instead set it from outside based on ProductService, since it inherently has nothing to do with the path resolving functionality.

@oakaigh
Copy link
Contributor Author

oakaigh commented Jan 17, 2024

PR moved to #202491

@aeschli
Copy link
Contributor

aeschli commented Feb 26, 2024

Implemented by #202491

Note that I changed the argument to --server-base-path

@aeschli aeschli closed this Feb 26, 2024
@microsoft microsoft locked and limited conversation to collaborators Jun 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants