-
Notifications
You must be signed in to change notification settings - Fork 1
add ws diagnose util to detect cyclic dependencies and multiple variants of the same dependency #17
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -37,6 +37,9 @@ log() { $QUIET || echo -e "$@"; } | |||||||||||||||||||||||||||||||||||||||||||||||||||||
| # has_flake: "yes" or "no" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| REPOS=( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Shared Nix infrastructure | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "logos-nix|logos-nix|https://github.com/logos-co/logos-nix.git|yes" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Foundation | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "logos-cpp-sdk|logos-cpp-sdk|https://github.com/logos-co/logos-cpp-sdk.git|yes" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "logos-module|logos-module|https://github.com/logos-co/logos-module.git|yes" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -1994,6 +1997,268 @@ HEADER | |||||||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cmd_nix_diagnose() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| wants_help "$@" && show_help "Usage: ws nix-diagnose <repo> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Diagnose dependency issues in a repo's flake. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Checks: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 1. Circular dependencies in the workspace dependency graph | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 2. Duplicate dependencies — same repo appearing at different versions | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| in the flake.lock (e.g. two copies of logos-nix at different revs) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Examples: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ws nix-diagnose logos-basecamp | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ws nix-diagnose logos-liblogos" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| local repo="" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| while [[ $# -gt 0 ]]; do | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| case "$1" in | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| -*) shift ;; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| *) repo="$1"; shift ;; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| esac | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| done | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if [[ -z "$repo" ]]; then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo -e "${RED}Error:${NC} repo name required" >&2 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| exit 1 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| local entry | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| entry=$(find_repo_entry "$repo") || { echo -e "${RED}Unknown repo:${NC} $repo" >&2; exit 1; } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| local input_name dir_name | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
| input_name=$(get_field "$entry" 1) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+2029
to
+2030
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
| local input_name dir_name | |
| input_name=$(get_field "$entry" 1) | |
| local dir_name |
Copilot
AI
Mar 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The jq extraction treats dependency edges as only those inputs whose value is a string, but in this repo’s flake.lock many inputs are arrays (e.g. ["nixpkgs"] or longer paths). This will drop most edges and can incorrectly report “No circular dependencies found”. Update the jq to also handle array inputs (e.g. map arrays to their first element / target node key) so the dependency graph is complete.
| select(.value | type == "string") | | |
| .value as $child | | |
| .value as $input | | |
| (if ($input | type) == "string" then | |
| $input | |
| elif ($input | type) == "array" and ($input | length) > 0 then | |
| $input[0] | |
| else | |
| empty | |
| end) as $child | |
Copilot
AI
Mar 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the cycle section, jq errors are suppressed (2>/dev/null || true), which can turn parse failures into an empty edge list and a misleading “No circular dependencies found”. Consider checking jq’s exit status and reporting a clear parse/error message instead of silently continuing.
Copilot
AI
Mar 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These grep patterns interpolate repo/node names directly into a regex (e.g. ^${node}|). Repo IDs / node keys can contain characters like . that are meaningful in regex, which can cause incorrect matches. Prefer fixed-string matching (grep -F) or escape the variables before using them in regex patterns.
Copilot
AI
Mar 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
start_repo is computed but never used. This looks like leftover scaffolding; consider removing it to avoid confusion and keep the cycle detection logic focused.
| local start_repo | |
| start_repo=$(jq -r ' | |
| .nodes.root.inputs | to_entries[] | | |
| select(.value | type == "string") | .key | |
| ' "$lock_file" 2>/dev/null | head -1 || true) |
Copilot
AI
Mar 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Root inputs are filtered to type == "string", but root inputs can also be arrays in flake.lock (path-form). That can make root_inputs incomplete/empty and skip traversal entirely. Consider normalizing input refs so both string and array forms are traversed.
| .nodes.root.inputs | to_entries[] | | |
| select(.value | type == "string") | .key | |
| ' "$lock_file" 2>/dev/null | head -1 || true) | |
| # Visit all repos reachable from root inputs | |
| local root_inputs | |
| root_inputs=$(jq -r ' | |
| .nodes.root.inputs | to_entries[] | | |
| select(.value | type == "string") | .value | |
| .nodes.root.inputs | |
| | to_entries[] | |
| | .value | |
| | if type == "string" then . | |
| elif type == "array" and length > 0 then .[0] | |
| else empty | |
| end | |
| ' "$lock_file" 2>/dev/null | head -1 || true) | |
| # Visit all repos reachable from root inputs | |
| local root_inputs | |
| root_inputs=$(jq -r ' | |
| .nodes.root.inputs | |
| | to_entries[] | |
| | .value | |
| | if type == "string" then . | |
| elif type == "array" and length > 0 then .[0] | |
| else empty | |
| end |
Copilot
AI
Mar 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The adjacency list for _find_path only includes inputs whose value is a string. In flake.lock, many inputs are arrays, so paths to conflicting nodes will often show as “(unreachable)” even when reachable. Normalize .value so both string and array forms contribute edges.
| select(.value | type == "string") | | |
| $parent + " " + .value + " " + .key | |
| .key as $input_name | | |
| .value as $v | | |
| (if ($v | type) == "string" then [$v] | |
| elif ($v | type) == "array" then $v | |
| else [] end)[] as $child | | |
| $parent + " " + $child + " " + $input_name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logos-nixis added to the REPOS registry, but it isn’t present in.gitmodulesyet. With this change,ws initwill try togit submodule addit in user workspaces, which creates local.gitmodules/index changes. Consider adding the submodule entry to.gitmodulesin this PR (or documenting why it should remain auto-added).