Add depot-tools meant for chromium development #2
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for installing and initializing depot_tools, a set of utilities required for Chromium development. The implementation registers depot_tools as a new optional component that can be enabled/disabled through configuration files. However, the PR also includes two unrelated utility installation scripts (tree and cmake) and removes a user-specific path from source control.
- Adds depot_tools component with installation script and shell initialization
- Updates all configuration YAML files to include the new depot_tools option
- Modifies CLI to register and handle depot_tools component installation
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/zsh/snippets | Removes accidentally committed file containing hardcoded user path |
| src/zsh/plugin-init/depot-tools.zsh | Adds shell initialization for depot_tools PATH setup (but not integrated into base.zsh) |
| src/setup/install-depot-tools.zsh | Implements depot_tools installation/update script |
| src/setup/install-tree.zsh | Unrelated: adds tree utility installer (missing shebang, not registered) |
| src/setup/install-cmake.zsh | Unrelated: adds cmake installer (missing shebang, not registered) |
| osa-cli.zsh | Registers depot-tools component and adds to component_keys arrays |
| configs/*.yaml | Adds depot_tools flag to all configuration presets |
Comments suppressed due to low confidence (1)
src/zsh/snippets:1
- This file should not be tracked in git. According to .gitignore line 10, the entire src/zsh/snippets/ directory is intentionally ignored because it contains downloaded snippets installed during setup. This file appears to contain a hardcoded user-specific path (/Users/fre/dev/osa-snippets) which should never be committed. The deletion is correct, but verify this wasn't accidentally committed in a previous change.
| @@ -0,0 +1,3 @@ | |||
| # Install cmake for building C/C++ projects | |||
There was a problem hiding this comment.
Missing shebang at the beginning of the file. All zsh script files must start with #!/usr/bin/env zsh according to the project's coding guidelines.
| #!/usr/bin/env zsh | ||
| # depot_tools plugin initialization | ||
| # Add depot_tools to PATH if installed | ||
| # https://commondatastorage.googleapis.com/chrome-infra-docs/flat/depot_tools/docs/html/depot_tools_tutorial.html | ||
|
|
||
| DEPOT_TOOLS_HOME="${DEPOT_TOOLS_HOME:-$HOME/.depot_tools}" | ||
|
|
||
| if [[ -d "$DEPOT_TOOLS_HOME" ]]; then | ||
| export PATH="$DEPOT_TOOLS_HOME:$PATH" | ||
| else | ||
| # Inform user depot_tools is not installed | ||
| if [[ -z "$DEPOT_TOOLS_SKIP_INIT_MESSAGE" ]]; then | ||
| echo "Note: depot_tools not found at: $DEPOT_TOOLS_HOME" | ||
| echo "Install with: ./osa-cli.zsh --enable depot-tools" | ||
| echo "Or manually: git clone https://chromium.googlesource.com/chromium/tools/depot_tools.git ~/.depot_tools" | ||
| export DEPOT_TOOLS_SKIP_INIT_MESSAGE=1 | ||
| fi | ||
| fi |
There was a problem hiding this comment.
The depot-tools plugin initialization file is created but never loaded in src/zsh/constructors/base.zsh. Similar to how android-sdk.zsh is conditionally loaded (line 46-48 in base.zsh), you need to add a conditional load for depot-tools.zsh. Without this, the depot-tools PATH setup will never take effect during shell initialization.
name: Pull Request
about: depot tools for chromium
title: ''
labels: ''
assignees: ''
Description
Type of Change
Release Management
For automatic release on merge, add one of these labels:
release:major- Breaking changes, incompatible API changes (bumps 1.x.x → 2.0.0)release:minor- New features, backwards-compatible (bumps x.1.x → x.2.0)No label = No automatic release (manual release via GitHub Actions later)
Testing
./osa-cli.zsh --scan-secrets(if touching constructors)./tests/run-tests.zshChecklist
#!/usr/bin/env zshshebang to new scriptsosa-secret-setinstead).zshextension (unless intentionally.shfor bash)release:majororrelease:minor)Additional Context