Fix override resolution ordering

resolver/resolver.go

@@ -123,14 +123,29 @@ func resolveSources(dotfiles dotfileMap, sources []string, group string) {
 
 // resolveOverrides scans a dotfiles map for files that have associated
 // override files. The override files will be removed as individual dotfiles
-// and their sources will be inserted into the file they are associated to.
+// and their sources will be inserted into the file they are associated to. Any
+// override dotfiels which do not override an existing dotfile will have a new
+// dotfile group created.
 func resolveOverrides(dotfiles dotfileMap, overrideSuffix string) {
+	flatten := map[string]string{}
+
 	for path, dotfile := range dotfiles {
-		if strings.HasSuffix(path, "."+overrideSuffix) {
+		if strings.HasSuffix(path, overrideSuffix) {
+			// If this override file has nothing to override mark it to be
+			// flattened (the suffix will be removed). We mark it instead of
+			// flattening it into the dotfiles map here as items added to maps
+			// may be itterated over if added while iterrating a map.
+			overridesPath := strings.TrimSuffix(path, overrideSuffix)
+
+			// Only if this override overides nothing
+			if _, ok := dotfiles[overridesPath]; !ok {
+				flatten[path] = overridesPath
+			}
+
 			continue
 		}
 
-		overridePath := path + "." + overrideSuffix
+		overridePath := path + overrideSuffix
 		overrideDotfile, exists := dotfiles[overridePath]
 
 		if !exists {
@@ -144,6 +159,18 @@ func resolveOverrides(dotfiles dotfileMap, overrideSuffix string) {
 		dotfile.Sources = append(dotfile.Sources, overrideDotfile.Sources...)
 		delete(dotfiles, overridePath)
 	}
+
+	for path, overridesPath := range flatten {
+		dotfiles[overridesPath] = dotfiles[path]
+		delete(dotfiles, path)
+
+		file := dotfiles[overridesPath]
+
+		// We can expect that only one source file should exist in the dotfiles
+		// source set. Mark it as an override source
+		file.Sources[0].Override = true
+		file.Path = strings.TrimSuffix(file.Path, overrideSuffix)
+	}
 }
 
 // resolveRemoved inserts entries into a dotfiles map for files that previously
@@ -191,10 +218,10 @@ func ResolveDotfiles(conf config.SourceConfig, lockfile config.SourceLockfile) D
 
 	for _, group := range groups {
 		resolveSources(dotfiles, sources, group)
+		resolveOverrides(dotfiles, "."+conf.OverrideSuffix)
 	}
 
 	resolveRemoved(dotfiles, lockfile.InstalledFiles)
-	resolveOverrides(dotfiles, conf.OverrideSuffix)
 
 	return dotfiles.asList()
 }

resolver/resolver_test.go

@@ -62,11 +62,12 @@ func TestResolveDotfiles(t *testing.T) {
 					},
 				},
 				{
-					Path: "blank.ovrd",
+					Path: "blank",
 					Sources: []*SourceFile{
 						{
-							Group: "machines/server",
-							Path:  "machines/server/blank.ovrd",
+							Group:    "machines/server",
+							Path:     "machines/server/blank.ovrd",
+							Override: true,
 						},
 					},
 				},
@@ -146,6 +147,87 @@ func TestResolveDotfiles(t *testing.T) {
 				},
 			},
 		},
+		// Override resolve order test. Override's should resolve as the groups
+		// are resolved.
+		{
+			CaseName: "Override resolve order",
+			SourceFiles: []string{
+				"base/bash/file1",
+				"machines/desktop/bash/file1.override",
+				"machines/server/bash/file1",
+				// Ensure resolving an override when there was no file to
+				// override does not override _later_ groups.
+				"base/bash/file2.override",
+				"machines/desktop/bash/file2",
+				// Ensure override files without any files to override have
+				// their override stripped.
+				"base/bash/file3.override",
+
+				"base/bash/file4.override",
+				"machines/desktop/bash/file4.override",
+			},
+			ExistingFiles: []string{},
+			Groups:        []string{"base", "machines/desktop", "machines/server"},
+			Expected: Dotfiles{
+				{
+					Path: "bash/file1",
+					Sources: []*SourceFile{
+						{
+							Group: "base",
+							Path:  "base/bash/file1",
+						},
+						{
+							Group:    "machines/desktop",
+							Path:     "machines/desktop/bash/file1.override",
+							Override: true,
+						},
+						{
+							Group: "machines/server",
+							Path:  "machines/server/bash/file1",
+						},
+					},
+				},
+				{
+					Path: "bash/file2",
+					Sources: []*SourceFile{
+						{
+							Group:    "base",
+							Path:     "base/bash/file2.override",
+							Override: true,
+						},
+						{
+							Group: "machines/desktop",
+							Path:  "machines/desktop/bash/file2",
+						},
+					},
+				},
+				{
+					Path: "bash/file3",
+					Sources: []*SourceFile{
+						{
+							Group:    "base",
+							Path:     "base/bash/file3.override",
+							Override: true,
+						},
+					},
+				},
+				{
+					Path: "bash/file4",
+					Sources: []*SourceFile{
+						{
+							Group:    "base",
+							Path:     "base/bash/file4.override",
+							Override: true,
+						},
+						{
+							Group:    "machines/desktop",
+							Path:     "machines/desktop/bash/file4.override",
+							Override: true,
+						},
+					},
+				},
+			},
+		},
 	}
 
 	ogSourceLoader := sourceLoader
@@ -181,7 +263,11 @@ func TestResolveDotfiles(t *testing.T) {
 		for _, dotfile := range dotfiles {
 			sourceFiles := []string{}
 			for _, source := range dotfile.Sources {
-				sourceFiles = append(sourceFiles, source.Path)
+				if source.Override {
+					sourceFiles = append(sourceFiles, source.Path+"[O]")
+				} else {
+					sourceFiles = append(sourceFiles, source.Path)
+				}
 			}
 
 			str := fmt.Sprintf("%s: [%s]", dotfile.Path, strings.Join(sourceFiles, ", "))