Skip to content

feat: add support for path conflicts with 'prefer' - #201

Merged
niemeyer merged 12 commits into
canonical:mainfrom
letFunny:prefer-tarjan-sort
Feb 11, 2025
Merged

niemeyer merged 12 commits into
canonical:mainfrom
letFunny:prefer-tarjan-sort

Conversation

@letFunny

Copy link
Copy Markdown
Collaborator

The "prefer" keyword is used to resolve path conflicts according to the RK021 path conflicts spec.

  • Have you signed the CLA?

The "prefer" keyword is used to resolve path conflicts according to the
RK021 path conflicts spec.
Comment thread internal/setup/setup.go
paths := make(map[string]*Slice)
for _, new := range selection.Slices {
for newPath, newInfo := range new.Contents {
if old, ok := paths[newPath]; ok {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Comment for reviewer]: This logic was not used anymore and was duplicating the conflict detection by doing it once when parsing the release and another time when getting the selection.

Comment thread internal/setup/setup.go Outdated
// having to do further processing.
nodes := make(map[string]*Slice)
successors := make(map[string][]string)
const (

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Comment for reviewer]: I thought about moving this state machine checks to the Tarjan Sort. However, I don't really see a benefit in doing that. It means that Tarjan Sort will need to keep track of whether paths are disconnected or have prefers (node can appear in any order if there are no edges). Additionally, it will be difficult to not deviate from the previous algorithm that was checking for SameContent inline. For example, how would we know if a node without prefers need to be checked because it is disconnected, or not because it is the tail of a prefers. Without the state machine, that is very hard to do.

@github-actions

github-actions Bot commented Feb 4, 2025 •

Copy link
Copy Markdown
Command Mean [s] Min [s] Max [s] Relative
BASE 7.304 ± 0.028 7.249 7.340 1.00
HEAD 7.309 ± 0.038 7.260 7.389 1.00 ± 0.01

@letFunny
letFunny requested a review from niemeyer February 5, 2025 11:04
@letFunny letFunny added the Priority Look at me first label Feb 5, 2025
@letFunny
letFunny force-pushed the prefer-tarjan-sort branch 4 times, most recently from 701ea5e to c634f95 Compare February 11, 2025 15:19

@niemeyer niemeyer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for curating this for integration, Alberto.

Only a couple of trivials.

Comment thread internal/setup/setup_test.go Resolved
Comment thread internal/setup/yaml.go Resolved
@niemeyer
niemeyer merged commit 19296fb into canonical:main Feb 11, 2025
@letFunny
letFunny deleted the prefer-tarjan-sort branch February 12, 2025 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority Look at me first

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants