fix: Improvements to DOT dependency graph generation#377
Merged
Conversation
- Add edge deduplication in rpmutils and debutils GenerateDot functions - Track written edges using map to prevent duplicate dependency links - Add test cases to verify each unique edge appears exactly once - Fixes issue where packages with multiple identical Requires entries created duplicate edges in DOT files (e.g., libstdc++ -> glibc appearing 5 times) Resolves duplicate dependency visualization in RPM and DEB package graphs. chore: remove accidentally committed binary and log file
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes duplicate dependency edges in DOT graph generation for RPM and DEB package dependency visualizations. The issue occurred when packages had multiple Requires entries that resolved to the same dependency, resulting in redundant edges like libstdc++ -> glibc appearing multiple times in the output file.
Changes:
- Added edge deduplication logic using a
map[string]boolto track written edges in bothrpmutils.GenerateDot()anddebutils.GenerateDot() - Added comprehensive test cases to verify each unique edge appears exactly once in generated DOT files
Reviewed changes
Copilot reviewed 4 out of 6 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/ospackage/rpmutils/resolver.go | Implemented edge deduplication in RPM DOT generation using edgesWritten map |
| internal/ospackage/rpmutils/resolver_test.go | Added test case with duplicate dependencies and verification logic for deduplication |
| internal/ospackage/debutils/resolver.go | Implemented edge deduplication in DEB DOT generation using edgesWritten map |
| internal/ospackage/debutils/resolver_test.go | Added test case with duplicate dependencies and verification logic for deduplication |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Change priority order: Essential (30) > Kernel/Bootloader (20) > System (10) - Previously System had highest priority (30) causing incorrect colors: * Packages in both SystemConfig.Packages and EssentialPkgList showed green * Essential packages should display yellow, not green - Essential and specific packages (kernel, bootloader) now correctly override generic system package classification - Reorder setSources calls to process from general to specific - Update test to reflect new priority where essential overrides system This ensures DOT graphs show correct semantic colors matching package roles. Note: Color coding for RPM packages still requires base name extraction fix (RPM filenames include version/arch, but pkgSources uses base names only)
Simplifies DOT file generation by removing package source color coding: - Removed color/fillcolor attributes from nodes - Removed legend generation (packageSourceStyles, legendOrder, writeLegend) - Simplified node output to basic format: "packagename"; - Updated tests to match simplified DOT format Rationale: - Color coding was complex and confusing - RPM color coding didn't work due to package name format mismatch - Priority system added unnecessary complexity - Simplified DOT files are easier to understand and maintain All tests pass (earthly +test-quick)
Remove all references to color coding system (Yellow/Green/Blue/Orange for Essential/System/Kernel/Bootloader packages) from documentation to match the simplified DOT graph implementation. Changes: - Remove 'Color Preservation' design principle from ADR - Remove 'Semantic Color Preservation' section with color table from ADR - Remove color-coded description from --dotfile flag in CLI specification - Remove color legend from --dotfile flag in main documentation This documentation update ensures consistency with the code changes that removed color coding complexity from dependency graph generation.
…ormat Fix false warning 'Root node not found' by updating the grep pattern to match both node declarations and edge statements. Since color attributes were removed, nodes now only appear in edge statements (node -> dep) rather than standalone declarations (node [attrs];). Also remove outdated color preservation notes from script header comments. Changes: - Update node detection pattern to match edge statements - Remove color scheme documentation from usage help - Remove 'Preserves existing semantic colors' comment
arodage
changed the title
Allow dep-analyzer.sh to render the complete dependency graph when --root is not provided. Previously, --root was always required, forcing users to slice the graph even when they wanted to visualize the entire dependency tree. Usage: ./dep-analyzer.sh -i deps.dot -t svg # Render full graph ./dep-analyzer.sh -i deps.dot -r vim # Slice to vim's dependencies Changes: - Make --root optional (only required for slicing mode) - Add full graph rendering when --root is omitted - Update usage documentation with full graph example - Clarify which options require --root
arodage
changed the title
Replace fmt.Sprintf() with simple string concatenation for edge keys in DOT generation. The previous format included spaces and arrows which were only needed for display, not for deduplication logic. Changes: - debutils: edgeKey = pkg.Name + "|" + depName - rpmutils: edgeKey = pkg.Name + "|" + dep This reduces string formatting overhead in the hot path of edge deduplication without changing functionality. Addresses Copilot PR review suggestions.
arodage
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merge Checklist
Description
This PR fixes issues with DOT file dependency graph generation for RPM and DEB packages.
Issue 1: Duplicate Dependency Edges
The DOT file generation was creating multiple identical edges for the same package dependency. For example,
libstdc++ -> glibcappeared 5 times in the azl3-minimal-deps.dot file when a package had multipleRequiresentries that resolved to the same dependency.Solution:
rpmutils.GenerateDot()anddebutils.GenerateDot()Issue 2: Color Coding removed due to Complexity
The DOT file color coding system was confusing and prone to errors:
Solution:
Issue 3: dep-analyzer Script Enhancement
Added support for rendering the complete dependency graph without requiring a root package for slicing. Previously, the
--rootparameter was mandatory, forcing users to specify a package even when they wanted to visualize the entire dependency tree.Solution:
--rootoptional - only required when slicing the graph--rootis omitted, the script now renders or copies the full graph--root(reverse, highlight-root)Usage:
This enhancement makes the tool more flexible and intuitive for users who want to visualize complete dependency graphs.
Documantation update:
Any Newly Introduced Dependencies
None
How Has This Been Tested?
earthly +test-quick- all tests passTest results:
libstdc++ -> glibcappeared 5 timeslibstdc++ -> glibcappears 1 time ✅Example visualization: