Conversation
There was a problem hiding this comment.
Pull request overview
Updates the workspace flake inputs/submodules and introduces a new ws repo-upgrade command to fetch the latest default-branch commits across repos.
Changes:
- Add
logos-nixas shared Nix infrastructure and rewire multiple flakefollowsto centralizenixpkgspinning. - Update
nix/dep-graph.nixdependencies to reflect new input relationships and removed circular deps. - Add
ws repo-upgradecommand plus documentation, and bump multiple submodule revisions.
Reviewed changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/ws | Adds repo-upgrade command to fetch/checkout latest default-branch commits for cloned repos. |
| flake.nix | Introduces logos-nix input and updates many inputs.*.follows to unify pinning and simplify deps. |
| nix/dep-graph.nix | Updates repo dependency graph to match new flake relationships. |
| README.md | Documents new ws repo-upgrade command. |
| repos/nix-bundle-dir | Bumps submodule revision. |
| repos/logos-webview-app | Bumps submodule revision. |
| repos/logos-wallet-ui | Bumps submodule revision. |
| repos/logos-wallet-module | Bumps submodule revision. |
| repos/logos-test-modules | Bumps submodule revision. |
| repos/logos-storage-module | Bumps submodule revision. |
| repos/logos-package | Bumps submodule revision. |
| repos/logos-nim-sdk | Bumps submodule revision. |
| repos/logos-module-builder | Bumps submodule revision. |
| repos/logos-module | Bumps submodule revision. |
| repos/logos-libp2p-module | Bumps submodule revision. |
| repos/logos-docs | Bumps submodule revision. |
| repos/logos-design-system | Bumps submodule revision. |
| repos/logos-chatsdk-ui | Bumps submodule revision. |
| repos/logos-chatsdk-module | Bumps submodule revision. |
| repos/logos-chat-ui | Bumps submodule revision. |
| repos/logos-chat-module | Bumps submodule revision. |
| repos/logos-capability-module | Bumps submodule revision. |
| repos/logos-accounts-ui | Bumps submodule revision. |
| repos/counter_qml | Bumps submodule revision. |
| repos/counter | Bumps submodule revision. |
| fi | ||
|
|
||
| local default_branch | ||
| default_branch=$(git -C "$repo_dir" symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@refs/remotes/origin/@@' || echo "master") |
There was a problem hiding this comment.
The || echo \"master\" fallback is unreliable because the pipeline’s exit status is typically that of sed, so a failing git symbolic-ref can still produce an empty default_branch (and you’ll attempt git fetch origin \"\"). Prefer capturing the command output and explicitly falling back when the resulting string is empty (or enable pipefail locally for this pipeline).
| default_branch=$(git -C "$repo_dir" symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@refs/remotes/origin/@@' || echo "master") | |
| default_branch=$(git -C "$repo_dir" symbolic-ref refs/remotes/origin/HEAD 2>/dev/null | sed 's@refs/remotes/origin/@@' || true) | |
| if [[ -z "$default_branch" ]]; then | |
| default_branch="master" | |
| fi |
| git -C "$repo_dir" checkout "origin/$default_branch" --detach 2>/dev/null | ||
| local short_rev | ||
| short_rev=$(git -C "$repo_dir" rev-parse --short HEAD 2>/dev/null) | ||
| log " ${GREEN}[$i/$total] $dir_name${NC} -> ${CYAN}$default_branch${NC} ($short_rev)" |
There was a problem hiding this comment.
The checkout result is not checked. If checkout fails (e.g., local changes prevent detach checkout), the command will still log success and compute a revision (possibly from the pre-existing HEAD). Capture and validate the exit code of the checkout (and/or rev-parse) and move the repo to failed[] when it fails.
| git -C "$repo_dir" checkout "origin/$default_branch" --detach 2>/dev/null | |
| local short_rev | |
| short_rev=$(git -C "$repo_dir" rev-parse --short HEAD 2>/dev/null) | |
| log " ${GREEN}[$i/$total] $dir_name${NC} -> ${CYAN}$default_branch${NC} ($short_rev)" | |
| if git -C "$repo_dir" checkout "origin/$default_branch" --detach 2>/dev/null; then | |
| local short_rev | |
| if ! short_rev=$(git -C "$repo_dir" rev-parse --short HEAD 2>/dev/null); then | |
| log " ${RED}[$i/$total] $dir_name — failed to determine revision${NC}" | |
| failed+=("$dir_name") | |
| else | |
| log " ${GREEN}[$i/$total] $dir_name${NC} -> ${CYAN}$default_branch${NC} ($short_rev)" | |
| fi | |
| else | |
| log " ${RED}[$i/$total] $dir_name — checkout failed${NC}" | |
| failed+=("$dir_name") | |
| fi |
| shift | ||
| done ;; | ||
| --all) all=true; shift ;; | ||
| -*) shift ;; |
There was a problem hiding this comment.
Unknown options (-*) are silently ignored, which can hide mistakes (e.g., typos) and lead to surprising behavior. Consider treating unknown flags as an error with a short message plus the usage string, consistent with other CLI tools.
| -*) shift ;; | |
| -*) | |
| echo -e "${RED}Unknown option:${NC} $1" >&2 | |
| echo "Usage: ws repo-upgrade [--group|-g GROUP ...] [--all] [REPO ...]" >&2 | |
| exit 1 | |
| ;; |
| failed+=("$dir_name") | ||
| fi | ||
|
|
||
| sleep $((RANDOM % 2 + 2)) |
There was a problem hiding this comment.
Unconditional per-repo sleep adds noticeable latency when upgrading many repos (2–3s * repo count). If this is intended as rate-limiting, it should be documented and/or made optional (e.g., only on retry, or behind a flag/env var); otherwise it’s best removed.
| sleep $((RANDOM % 2 + 2)) | |
| # Optional per-repo rate limiting; override/disable via WS_REPO_UPGRADE_SLEEP (e.g., 0 to skip). | |
| if [[ -n "${WS_REPO_UPGRADE_SLEEP:-}" ]]; then | |
| sleep "${WS_REPO_UPGRADE_SLEEP}" | |
| else | |
| sleep $((RANDOM % 2 + 2)) | |
| fi |
nix/dep-graph.nix
Outdated
| logos-cpp-sdk = { deps = []; hasTests = false; }; | ||
| logos-module = { deps = [ "logos-cpp-sdk" ]; hasTests = true; }; | ||
| logos-liblogos = { deps = [ "logos-cpp-sdk" "logos-module" "logos-capability-module" "nix-bundle-dir" ]; hasTests = true; }; | ||
| logos-capability-module = { deps = [ "logos-cpp-sdk" "logos-liblogos" ]; hasTests = false; }; | ||
| logos-package = { deps = [ "logos-liblogos" ]; hasTests = true; }; | ||
| logos-module-builder = { deps = [ "logos-cpp-sdk" "logos-liblogos" ]; hasTests = false; }; | ||
| nix-bundle-dir = { deps = []; hasTests = false; }; | ||
| nix-bundle-lgx = { deps = [ "logos-package" "nix-bundle-dir" ]; hasTests = false; }; | ||
| logos-basecamp = { deps = [ "logos-cpp-sdk" "logos-liblogos" "logos-capability-module" "logos-package" "nix-bundle-dir" "nix-bundle-lgx" "logos-package-manager-module" "logos-package-manager-ui" "logos-webview-app" "counter_qml" "counter" "logos-design-system" ]; hasTests = true; }; | ||
| logos-module = { deps = [ "logos-nix" ]; hasTests = true; }; | ||
| logos-liblogos = { deps = [ "logos-nix" "logos-cpp-sdk" "logos-module" "logos-capability-module" "nix-bundle-dir" ]; hasTests = true; }; | ||
| logos-capability-module = { deps = [ "logos-nix" "logos-cpp-sdk" "logos-module" ]; hasTests = false; }; | ||
| logos-package = { deps = [ "logos-nix" ]; hasTests = true; }; | ||
| logos-module-builder = { deps = [ "logos-nix" "logos-cpp-sdk" "logos-module" ]; hasTests = false; }; | ||
| nix-bundle-dir = { deps = [ "logos-nix" ]; hasTests = false; }; | ||
| nix-bundle-lgx = { deps = [ "logos-nix" "logos-package" "nix-bundle-dir" ]; hasTests = false; }; | ||
| logos-basecamp = { deps = [ "logos-nix" "logos-cpp-sdk" "logos-liblogos" "logos-capability-module" "logos-package" "nix-bundle-dir" "nix-bundle-lgx" "logos-package-manager-module" "logos-package-manager-ui" "logos-webview-app" "counter_qml" "counter" "logos-design-system" ]; hasTests = true; }; |
There was a problem hiding this comment.
logos-nix is referenced as a dependency in multiple entries but is not defined as a node in the graph (at least in the shown attrset). If the graph tooling expects all deps to be present as keys, this will break graph traversal / topological ordering. Add a logos-nix = { deps = []; hasTests = false; }; entry (or equivalent) to the graph.
| inputs.nixpkgs.follows = "nixpkgs"; | ||
| inputs.logos-module-builder.follows = "logos-module-builder"; | ||
| inputs.logos-liblogos.follows = "logos-liblogos"; | ||
| inputs.logos-package-manager.follows = "logos-package-manager-module"; |
There was a problem hiding this comment.
The logos-test-modules follows entry uses inputs.logos-package-manager.follows, but elsewhere the repo is consistently named logos-package-manager-module. If the upstream logos-test-modules flake input is actually named logos-package-manager-module, this follows will not apply (and you can end up with duplicate versions). Align the input key to the actual upstream input name (likely inputs.logos-package-manager-module.follows = \"logos-package-manager-module\";).
| inputs.logos-package-manager.follows = "logos-package-manager-module"; | |
| inputs.logos-package-manager-module.follows = "logos-package-manager-module"; |
| | `ws worktree add <name> [-b br]` | Create a worktree with submodules and `ws/<branch>` branches | | ||
| | `ws worktree list` | List all worktrees | | ||
| | `ws worktree remove <name>` | Remove a worktree | | ||
| | `ws repo-upgrade [repo...\|--group G]` | Pull latest master/main for submodules | |
There was a problem hiding this comment.
The README usage omits options supported by the implementation/help (--all and -g, plus the default behavior when no args are provided). Update the README command signature to match scripts/ws help text so users don’t miss supported flags.
| | `ws repo-upgrade [repo...\|--group G]` | Pull latest master/main for submodules | | |
| | `ws repo-upgrade [repo...\|--group G\|--all]` | Pull latest master/main for submodules (default: all repos) | |
| done | ||
| } | ||
|
|
||
| cmd_repo_upgrade() { |
There was a problem hiding this comment.
The PR title is "Flake update", but this change set also introduces a new CLI command (ws repo-upgrade) and updates README/help output. Consider updating the PR title/description to reflect that it includes CLI surface-area changes (not just flake/submodule updates).
add cache
No description provided.