move canonicalize logic to dedicated function & run before and after …

…edit

Signed-off-by: Tim Ramlot <42113979+inteon@users.noreply.github.com>

pkg/mod/index.go

@@ -4,7 +4,7 @@ import (
  "bufio"
  "io"
  "path/filepath"
- "sort"
+ "slices"
  "strings"
 
  "github.com/rogpeppe/go-internal/lockedfile"
@@ -19,15 +19,42 @@ type kloneFile struct {
  Targets map[string]KloneFolder `yaml:"targets"`
 }
 
+func (f *kloneFile) canonicalize() {
+ newModTargets := make(map[string]KloneFolder, len(f.Targets))
+ for target, srcs := range f.Targets {
+ // Deduplicate sources based on cleaned relative path
+ uniqueSrcs := make(map[string]KloneItem, len(srcs))
+ for _, src := range srcs {
+ src.FolderName = cleanRelativePath(src.FolderName)
+ uniqueSrcs[src.FolderName] = src
+ }
+
+ // Rebuild array of sources, now without duplicates
+ srcs := make(KloneFolder, 0, len(uniqueSrcs))
+ for _, src := range uniqueSrcs {
+ srcs = append(srcs, src)
+ }
+
+ // Sort sources by folder name
+ slices.SortFunc(srcs, func(a, b KloneItem) int {
+ return a.Compare(b)
+ })
+
+ newModTargets[cleanRelativePath(target)] = srcs
+ }
+
+ f.Targets = newModTargets
+}
+
 type KloneFolder []KloneItem
 
 type KloneItem struct {
  FolderName string `yaml:"folder_name"`
  KloneSource `yaml:",inline"`
 }
 
-func (i KloneItem) Less(other KloneItem) bool {
- return i.FolderName < other.FolderName
+func (i KloneItem) Compare(other KloneItem) int {
+ return strings.Compare(i.FolderName, other.FolderName)
 }
 
 type KloneSource struct {
@@ -54,11 +81,17 @@ func (w WorkDir) editKloneFile(fn func(*kloneFile) error) error {
  return err
  }
 
+ // canonicalize index
+ index.canonicalize()
+
  // update index
  if err := fn(&index); err != nil {
  return err
  }
 
+ // canonicalize index
+ index.canonicalize()
+
  topComments := ""
 
  {
@@ -138,24 +171,20 @@ func (w WorkDir) readKloneFile() (*kloneFile, error) {
  return nil, err
  }
 
+ // canonicalize index
+ index.canonicalize()
+
  return &index, nil
 }
 
 func (w WorkDir) Init() error {
  return w.editKloneFile(func(kf *kloneFile) error {
- if kf.Targets == nil {
- kf.Targets = make(map[string]KloneFolder)
- }
  return nil
  })
 }
 
 func (w WorkDir) AddTarget(target string, folderName string, dep KloneSource) error {
  return w.editKloneFile(func(kf *kloneFile) error {
- if kf.Targets == nil {
- kf.Targets = make(map[string]KloneFolder)
- }
-
  for _, src := range kf.Targets[target] {
  if src.FolderName == folderName {
  src.KloneSource = dep
@@ -181,44 +210,21 @@ func (w WorkDir) FetchTargets(
  fetchFn func(target string, srcs KloneFolder) error,
 ) error {
  return w.editKloneFile(func(kf *kloneFile) error {
- // clean target paths (only keep the last entry if they are identical)
- newModTargets := make(map[string]KloneFolder, len(kf.Targets))
  for target, srcs := range kf.Targets {
- cleanedTarget := cleanRelativePath(target)
-
- newSrcs := make(map[string]KloneItem, len(srcs))
- for _, src := range srcs {
- src.FolderName = cleanRelativePath(src.FolderName)
-
- newSrcs[src.FolderName] = src
- }
-
- newSrcsArray := make(KloneFolder, 0, len(newSrcs))
- for _, src := range newSrcs {
- newSrcsArray = append(newSrcsArray, src)
- }
-
- sort.Slice(newSrcsArray, func(i, j int) bool {
- return newSrcsArray[i].Less(newSrcsArray[j])
- })
-
- for i, src := range newSrcsArray {
+ for i, src := range srcs {
  if err := cleanFn(target, src.FolderName, &src.KloneSource); err != nil {
  return err
  }
-
- newSrcsArray[i] = src
+ srcs[i] = src
  }
 
- if err := fetchFn(target, newSrcsArray); err != nil {
+ if err := fetchFn(target, srcs); err != nil {
  return err
  }
 
- newModTargets[cleanedTarget] = newSrcsArray
+ kf.Targets[target] = srcs
  }
 
- kf.Targets = newModTargets
-
  return nil
  })
 }

pkg/mod/index_test.go

@@ -3,7 +3,7 @@ package mod
 import (
  "os"
  "path"
- "sort"
+ "slices"
  "testing"
 )
 
@@ -16,8 +16,8 @@ func TestKloneItemSorting(t *testing.T) {
  }
 
  // Sort the items
- sort.Slice(items, func(i, j int) bool {
- return items[i].Less(items[j])
+ slices.SortFunc(items, func(a, b KloneItem) int {
+ return a.Compare(b)
  })
 
  // Verify the sorting order
@@ -61,6 +61,82 @@ targets:
  repo_ref: main
  repo_hash: abc123
  repo_path: path/to/repo1
+`,
+ expectErr: false,
+ },
+ {
+ name: "Sort targets (level 1)",
+ initial: `# Test comment1
+# Test comment2
+targets:
+ target2:
+ - folder_name: Folder A
+ repo_url: https://github.com/repo1
+ repo_ref: main
+ repo_hash: abc123
+ repo_path: path/to/repo1
+ target1:
+ - folder_name: Folder A
+ repo_url: https://github.com/repo1
+ repo_ref: main
+ repo_hash: abc123
+ repo_path: path/to/repo1
+`,
+ modifyFn: func(kf *kloneFile) error {
+ return nil
+ },
+ expected: `# Test comment1
+# Test comment2
+targets:
+ target1:
+ - folder_name: Folder A
+ repo_url: https://github.com/repo1
+ repo_ref: main
+ repo_hash: abc123
+ repo_path: path/to/repo1
+ target2:
+ - folder_name: Folder A
+ repo_url: https://github.com/repo1
+ repo_ref: main
+ repo_hash: abc123
+ repo_path: path/to/repo1
+`,
+ expectErr: false,
+ },
+ {
+ name: "Sort targets (level 2)",
+ initial: `# Test comment1
+# Test comment2
+targets:
+ target1:
+ - folder_name: Folder B
+ repo_url: https://github.com/repo1
+ repo_ref: main
+ repo_hash: abc123
+ repo_path: path/to/repo1
+ - folder_name: Folder A
+ repo_url: https://github.com/repo1
+ repo_ref: main
+ repo_hash: abc123
+ repo_path: path/to/repo1
+`,
+ modifyFn: func(kf *kloneFile) error {
+ return nil
+ },
+ expected: `# Test comment1
+# Test comment2
+targets:
+ target1:
+ - folder_name: Folder A
+ repo_url: https://github.com/repo1
+ repo_ref: main
+ repo_hash: abc123
+ repo_path: path/to/repo1
+ - folder_name: Folder B
+ repo_url: https://github.com/repo1
+ repo_ref: main
+ repo_hash: abc123
+ repo_path: path/to/repo1
 `,
  expectErr: false,
  },