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

Clean up cli build #178691

Closed
3 tasks
joaomoreno opened this issue Mar 30, 2023 · 1 comment · Fixed by #190213
Closed
3 tasks

Clean up cli build #178691

joaomoreno opened this issue Mar 30, 2023 · 1 comment · Fixed by #190213
Assignees
Labels
engineering VS Code - Build / issue tracking / etc. insiders-released Patch has been released in VS Code Insiders
Milestone

Comments

@joaomoreno
Copy link
Member

There are a few strange choices in the cli build infrastructure:

As rules of thumb:

  • I should be able to just run cargo build and have an OSS build of the CLI, independent of the fact that I have a vscode-distro folder as a sibling of vscode.
  • After mixing in distro, no other component in our build infrastructure should ever reach into the quality folder.
    • If there is a reason for the Insider CLI (eg.) to know specific details about the stable and exploration qualities, this should be information contained in Insider's product.json itself, not done via some build magic which reaches out to other qualities product.json files.
  • There should be zero build components reaching out to build/azure-pipelines. Only Azure Pipelines should reach in there.
@joaomoreno joaomoreno added the engineering VS Code - Build / issue tracking / etc. label Mar 30, 2023
@connor4312
Copy link
Member

The nice thing about having build.rs look for vscode distro, is that it's then easy to locally build a CLI that supports tunnel access, which requires quality information from distros to function.

If there is a reason for the Insider CLI (eg.) to know specific details about the stable and exploration qualities, this should be information contained in Insider's product.json itself

This is currently necessary because, for example, the "stable" build of the CLI is able to launch Insiders servers when connected to from insiders.vscode.dev or an Insiders quality on desktop. Likewise, in its 'redirection' logic, the CLI can be configured to open a chosen quality e.g. ./code --quality [stable|insiders|exploration] .. You could argue this shouldn't be the case, but while they are, then we either need to pull this information from all qualities or duplicate all these things in each product.json--and make sure to keep them in sync!

/// Map of quality names to arrays of app IDs used for them, for example, `{"stable":["ABC123"]}`
pub static ref WIN32_APP_IDS: Option<HashMap<Quality, Vec<String>>> =
option_env!("VSCODE_CLI_WIN32_APP_IDS").and_then(|s| serde_json::from_str(s).unwrap());
/// Map of quality names to desktop download URIs
pub static ref QUALITY_DOWNLOAD_URIS: Option<HashMap<Quality, String>> =
option_env!("VSCODE_CLI_QUALITY_DOWNLOAD_URIS").and_then(|s| serde_json::from_str(s).unwrap());
/// Map of qualities to the long name of the app in that quality
pub static ref PRODUCT_NAME_LONG_MAP: Option<HashMap<Quality, String>> =
option_env!("VSCODE_CLI_NAME_LONG_MAP").and_then(|s| serde_json::from_str(s).unwrap());
/// Map of qualities to the application name
pub static ref APPLICATION_NAME_MAP: Option<HashMap<Quality, String>> =
option_env!("VSCODE_CLI_APPLICATION_NAME_MAP").and_then(|s| serde_json::from_str(s).unwrap());
/// Map of qualities to the server name
pub static ref SERVER_NAME_MAP: Option<HashMap<Quality, String>> =
option_env!("VSCODE_CLI_SERVER_NAME_MAP").and_then(|s| serde_json::from_str(s).unwrap());

I didn't/don't know of a reason why the build shouldn't be able to know about qualities other than the one currently being built, so the former option seemed the better choice.

connor4312 added a commit that referenced this issue Aug 9, 2023
- Remove the `prepare` script entirely
- Variables are now populated from the product.json during build. Most
  variables are mapped automatically, with some special handling in a
	few cases. `build.rs` is now much more self-contained.
- Look for the `product.overrides.json` for vscode developers instead of
  looking for a peer `vscode-distro` folder

Fixes #178691
connor4312 added a commit that referenced this issue Aug 9, 2023
- Remove the `prepare` script entirely
- Variables are now populated from the product.json during build. Most
  variables are mapped automatically, with some special handling in a
	few cases. `build.rs` is now much more self-contained.
- Look for the `product.overrides.json` for vscode developers instead of
  looking for a peer `vscode-distro` folder

Fixes #178691
connor4312 added a commit that referenced this issue Aug 9, 2023
- Remove the `prepare` script entirely
- Variables are now populated from the product.json during build. Most
  variables are mapped automatically, with some special handling in a
	few cases. `build.rs` is now much more self-contained.
- Look for the `product.overrides.json` for vscode developers instead of
  looking for a peer `vscode-distro` folder

Fixes #178691
connor4312 added a commit that referenced this issue Aug 9, 2023
- Remove the `prepare` script entirely
- Variables are now populated from the product.json during build. Most
  variables are mapped automatically, with some special handling in a
	few cases. `build.rs` is now much more self-contained.
- Look for the `product.overrides.json` for vscode developers instead of
  looking for a peer `vscode-distro` folder

Fixes #178691
connor4312 added a commit that referenced this issue Aug 9, 2023
- Remove the `prepare` script entirely
- Variables are now populated from the product.json during build. Most
  variables are mapped automatically, with some special handling in a
	few cases. `build.rs` is now much more self-contained.
- Look for the `product.overrides.json` for vscode developers instead of
  looking for a peer `vscode-distro` folder

Fixes #178691
connor4312 added a commit that referenced this issue Aug 9, 2023
- Remove the `prepare` script entirely
- Variables are now populated from the product.json during build. Most
  variables are mapped automatically, with some special handling in a
	few cases. `build.rs` is now much more self-contained.
- Look for the `product.overrides.json` for vscode developers instead of
  looking for a peer `vscode-distro` folder

Fixes #178691
connor4312 added a commit that referenced this issue Aug 9, 2023
- Remove the `prepare` script entirely
- Variables are now populated from the product.json during build. Most
  variables are mapped automatically, with some special handling in a
	few cases. `build.rs` is now much more self-contained.
- Look for the `product.overrides.json` for vscode developers instead of
  looking for a peer `vscode-distro` folder

Fixes #178691
connor4312 added a commit that referenced this issue Aug 9, 2023
- Remove the `prepare` script entirely
- Variables are now populated from the product.json during build. Most
  variables are mapped automatically, with some special handling in a
	few cases. `build.rs` is now much more self-contained.
- Look for the `product.overrides.json` for vscode developers instead of
  looking for a peer `vscode-distro` folder

Fixes #178691
connor4312 added a commit that referenced this issue Aug 10, 2023
- Remove the `prepare` script entirely
- Variables are now populated from the product.json during build. Most
  variables are mapped automatically, with some special handling in a
	few cases. `build.rs` is now much more self-contained.
- Look for the `product.overrides.json` for vscode developers instead of
  looking for a peer `vscode-distro` folder

Fixes #178691
connor4312 added a commit that referenced this issue Aug 10, 2023
- Remove the `prepare` script entirely
- Variables are now populated from the product.json during build. Most
  variables are mapped automatically, with some special handling in a
	few cases. `build.rs` is now much more self-contained.
- Look for the `product.overrides.json` for vscode developers instead of
  looking for a peer `vscode-distro` folder

Fixes #178691
connor4312 added a commit that referenced this issue Aug 10, 2023
- Remove the `prepare` script entirely
- Variables are now populated from the product.json during build. Most
  variables are mapped automatically, with some special handling in a
	few cases. `build.rs` is now much more self-contained.
- Look for the `product.overrides.json` for vscode developers instead of
  looking for a peer `vscode-distro` folder

Fixes #178691
connor4312 added a commit that referenced this issue Aug 10, 2023
- Remove the `prepare` script entirely
- Variables are now populated from the product.json during build. Most
  variables are mapped automatically, with some special handling in a
	few cases. `build.rs` is now much more self-contained.
- Look for the `product.overrides.json` for vscode developers instead of
  looking for a peer `vscode-distro` folder

Fixes #178691
connor4312 added a commit that referenced this issue Aug 10, 2023
- Remove the `prepare` script entirely
- Variables are now populated from the product.json during build. Most
  variables are mapped automatically, with some special handling in a
	few cases. `build.rs` is now much more self-contained.
- Look for the `product.overrides.json` for vscode developers instead of
  looking for a peer `vscode-distro` folder

Fixes #178691
connor4312 added a commit that referenced this issue Aug 10, 2023
- Remove the `prepare` script entirely
- Variables are now populated from the product.json during build. Most
  variables are mapped automatically, with some special handling in a
	few cases. `build.rs` is now much more self-contained.
- Look for the `product.overrides.json` for vscode developers instead of
  looking for a peer `vscode-distro` folder

Fixes #178691
connor4312 added a commit that referenced this issue Aug 10, 2023
- Remove the `prepare` script entirely
- Variables are now populated from the product.json during build. Most
  variables are mapped automatically, with some special handling in a
	few cases. `build.rs` is now much more self-contained.
- Look for the `product.overrides.json` for vscode developers instead of
  looking for a peer `vscode-distro` folder

Fixes #178691
connor4312 added a commit that referenced this issue Aug 10, 2023
- Remove the `prepare` script entirely
- Variables are now populated from the product.json during build. Most
  variables are mapped automatically, with some special handling in a
	few cases. `build.rs` is now much more self-contained.
- Look for the `product.overrides.json` for vscode developers instead of
  looking for a peer `vscode-distro` folder

Fixes #178691
connor4312 added a commit that referenced this issue Aug 10, 2023
- Remove the `prepare` script entirely
- Variables are now populated from the product.json during build. Most
  variables are mapped automatically, with some special handling in a
	few cases. `build.rs` is now much more self-contained.
- Look for the `product.overrides.json` for vscode developers instead of
  looking for a peer `vscode-distro` folder

Fixes #178691
@connor4312 connor4312 added this to the August 2023 milestone Aug 10, 2023
connor4312 added a commit that referenced this issue Aug 11, 2023
- Remove the `prepare` script entirely
- Variables are now populated from the product.json during build. Most
  variables are mapped automatically, with some special handling in a
	few cases. `build.rs` is now much more self-contained.
- Look for the `product.overrides.json` for vscode developers instead of
  looking for a peer `vscode-distro` folder

Fixes #178691
@vscodenpa vscodenpa added unreleased Patch has not yet been released in VS Code Insiders insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Aug 11, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Sep 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
engineering VS Code - Build / issue tracking / etc. insiders-released Patch has been released in VS Code Insiders
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants