Fix for policy manager not setting labels in some cases

snapshot/policy/policy_manager.go

@@ -3,7 +3,6 @@ package policy
 
 import (
 	"context"
-	"path/filepath"
 	"strings"
 
 	"github.com/pkg/errors"
@@ -36,7 +35,7 @@ func GetEffectivePolicy(ctx context.Context, rep repo.Repository, si snapshot.So
 
 		md = append(md, manifests...)
 
-		parentPath := filepath.Dir(tmp.Path)
+		parentPath := getParentPathOSIndependent(tmp.Path)
 		if parentPath == tmp.Path {
 			break
 		}
@@ -192,6 +191,15 @@ func (m SubdirectoryPolicyMap) GetPolicyForPath(relativePath string) (*Policy, e
 
 // TreeForSource returns policy Tree for a given source.
 func TreeForSource(ctx context.Context, rep repo.Repository, si snapshot.SourceInfo) (*Tree, error) {
+	pols, err := applicablePoliciesForSource(ctx, rep, si)
+	if err != nil {
+		return nil, errors.Wrap(err, "unable to get policies")
+	}
+
+	return BuildTree(pols, DefaultPolicy), nil
+}
+
+func applicablePoliciesForSource(ctx context.Context, rep repo.Repository, si snapshot.SourceInfo) (map[string]*Policy, error) {
 	result := map[string]*Policy{}
 
 	pol, _, err := GetEffectivePolicy(ctx, rep, si)
@@ -212,7 +220,7 @@ func TreeForSource(ctx context.Context, rep repo.Repository, si snapshot.SourceI
 		return nil, errors.Wrapf(err, "unable to find manifests for %v@%v", si.UserName, si.Host)
 	}
 
-	log(ctx).Debugf("found %v policies for %v@%v", si.UserName, si.Host)
+	log(ctx).Debugf("found %v policies for %v@%v", len(policies), si.UserName, si.Host)
 
 	for _, id := range policies {
 		pol := &Policy{}
@@ -224,22 +232,19 @@ func TreeForSource(ctx context.Context, rep repo.Repository, si snapshot.SourceI
 
 		policyPath := pol.Labels["path"]
 
-		if !strings.HasPrefix(policyPath, si.Path+"/") {
+		rel := nestedRelativePathNormalizedToSlashes(si.Path, policyPath)
+		if rel == "" {
+			log(ctx).Debugf("%v is not under %v", policyPath, si.Path)
 			continue
 		}
 
-		rel, err := filepath.Rel(si.Path, policyPath)
-		if err != nil {
-			return nil, errors.Wrap(err, "unable to determine relative path")
-		}
-
 		rel = "./" + rel
 		log(ctx).Debugf("loading policy for %v (%v)", policyPath, rel)
 
 		result[rel] = pol
 	}
 
-	return BuildTree(result, DefaultPolicy), nil
+	return result, nil
 }
 
 func loadPolicyFromManifest(ctx context.Context, rep repo.Repository, id manifest.ID, pol *Policy) error {
@@ -288,3 +293,65 @@ func labelsForSource(si snapshot.SourceInfo) map[string]string {
 		}
 	}
 }
+
+func getParentPathOSIndependent(p string) string {
+	// split into volume (Windows only, e.g. X:) and path using either slash or backslash.
+	vol, pth := volumeAndPath(p)
+
+	last := strings.LastIndexAny(pth, "/\\")
+	if last == len(pth)-1 && last != 0 {
+		pth = pth[0:last]
+		last = strings.LastIndexAny(pth, "/\\")
+	}
+
+	if last < 0 {
+		return p
+	}
+
+	// special case for root, return root path itself (either slash or backslash-separated)
+	if last == 0 {
+		return vol + pth[0:1]
+	}
+
+	return vol + pth[0:last]
+}
+
+// volumeAndPath splits path 'p' into Windows-specific volume (e.g. "X:" and path after that starting with either slash or backslash)
+func volumeAndPath(p string) (vol, path string) {
+	if len(p) >= 3 && p[1] == ':' && isSlashOrBackslash(p[2]) {
+		// "X:\"
+		return p[0:2], p[2:]
+	}
+
+	return "", p
+}
+
+func isSlashOrBackslash(c uint8) bool {
+	return c == '/' || c == '\\'
+}
+
+func isWindowsStylePath(p string) bool {
+	v, _ := volumeAndPath(p)
+	return v != ""
+}
+
+func trimTrailingSlashOrBackslash(path string) string {
+	return strings.TrimSuffix(strings.TrimSuffix(path, "/"), "\\")
+}
+
+func nestedRelativePathNormalizedToSlashes(parent, child string) string {
+	isWin := isWindowsStylePath(parent)
+	parent = trimTrailingSlashOrBackslash(parent)
+
+	if !strings.HasPrefix(child, parent+"/") && !strings.HasPrefix(child, parent+"\\") {
+		return ""
+	}
+
+	p := strings.TrimPrefix(child, parent)[1:]
+
+	if isWin {
+		return strings.ReplaceAll(p, "\\", "/")
+	}
+
+	return p
+}

snapshot/policy/policy_manager_test.go

@@ -4,11 +4,13 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"sort"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
 
 	"github.com/kopia/kopia/internal/repotesting"
+	"github.com/kopia/kopia/internal/testlogging"
 	"github.com/kopia/kopia/snapshot"
 )
 
@@ -199,6 +201,201 @@ func TestPolicyManagerResolvesConflicts(t *testing.T) {
 	}
 }
 
+func TestParentPathOSIndependent(t *testing.T) {
+	cases := []struct {
+		input, want string
+	}{
+		{"/", "/"},
+		{"/x", "/"},
+		{"/x/", "/"},
+		{"/x/a", "/x"},
+		{"/x/a/", "/x"},
+		{"x:\\", "x:\\"},
+		{"X:\\Program Files", "X:\\"},
+		{"X:\\Program Files\\Blah", "X:\\Program Files"},
+		{"X:/Program Files", "X:/"},
+		{"X:/Program Files/", "X:/"},
+		{"X:/Program Files\\", "X:/"},
+		{"X:/Program Files/Blah", "X:/Program Files"},
+		{"X:/Program Files/Blah/", "X:/Program Files"},
+		{"X:/Program Files/Blah/xxx", "X:/Program Files/Blah"},
+	}
+
+	for _, tc := range cases {
+		if got, want := getParentPathOSIndependent(tc.input), tc.want; got != want {
+			t.Errorf("invalid value of getParentPathOSIndependent(%q): %q, want %q", tc.input, got, want)
+		}
+	}
+}
+
+// TestApplicablePoliciesForSource verifies that when we build a policy tree, we pick the appropriate policies
+// defined for the subtree regardless of path style (Unix or Windows).
+func TestApplicablePoliciesForSource(t *testing.T) {
+	ctx := testlogging.Context(t)
+
+	var env repotesting.Environment
+
+	defer env.Setup(t).Close(ctx, t)
+
+	setPols := map[snapshot.SourceInfo]*Policy{
+		// unix-style path names
+		snapshot.SourceInfo{Host: "host-a"}: &Policy{
+			RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(0)},
+		},
+		snapshot.SourceInfo{Host: "host-a", UserName: "myuser", Path: "/home"}: &Policy{
+			RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(1)},
+		},
+		snapshot.SourceInfo{Host: "host-a", UserName: "myuser", Path: "/home/users"}: &Policy{
+			RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(2)},
+		},
+		snapshot.SourceInfo{Host: "host-a", UserName: "myuser", Path: "/home/users/myuser"}: &Policy{
+			RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(3)},
+		},
+		snapshot.SourceInfo{Host: "host-a", UserName: "myuser", Path: "/home/users/myuser/dir1"}: &Policy{
+			RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(4)},
+		},
+		snapshot.SourceInfo{Host: "host-a", UserName: "myuser", Path: "/home/users/myuser/dir2"}: &Policy{
+			RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(5)},
+		},
+		snapshot.SourceInfo{Host: "host-a", UserName: "myuser", Path: "/home/users/myuser/dir2/a"}: &Policy{
+			RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(6)},
+		},
+		snapshot.SourceInfo{Host: "host-a", UserName: "myuser", Path: "/home/users/myuser2"}: &Policy{
+			RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(7)},
+		},
+
+		// windows-style path names with backslash
+		snapshot.SourceInfo{Host: "host-b"}: &Policy{
+			RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(0)},
+		},
+		snapshot.SourceInfo{Host: "host-b", UserName: "myuser", Path: "C:\\"}: &Policy{
+			RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(1)},
+		},
+		snapshot.SourceInfo{Host: "host-b", UserName: "myuser", Path: "C:\\Users"}: &Policy{
+			RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(2)},
+		},
+		snapshot.SourceInfo{Host: "host-b", UserName: "myuser", Path: "C:\\Users\\myuser"}: &Policy{
+			RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(3)},
+		},
+		snapshot.SourceInfo{Host: "host-b", UserName: "myuser", Path: "C:\\Users\\myuser\\dir1"}: &Policy{
+			RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(4)},
+		},
+		snapshot.SourceInfo{Host: "host-b", UserName: "myuser", Path: "C:\\Users\\myuser\\dir2"}: &Policy{
+			RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(5)},
+		},
+		snapshot.SourceInfo{Host: "host-b", UserName: "myuser", Path: "C:\\Users\\myuser\\dir2\\a"}: &Policy{
+			RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(6)},
+		},
+		snapshot.SourceInfo{Host: "host-b", UserName: "myuser", Path: "C:\\Users\\myuser2"}: &Policy{
+			RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(7)},
+		},
+
+		// windows-style path names with slashes
+		snapshot.SourceInfo{Host: "host-c"}: &Policy{
+			RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(0)},
+		},
+		snapshot.SourceInfo{Host: "host-c", UserName: "myuser", Path: "C:/Users"}: &Policy{
+			RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(1)},
+		},
+		snapshot.SourceInfo{Host: "host-c", UserName: "myuser", Path: "C:/Users"}: &Policy{
+			RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(2)},
+		},
+		snapshot.SourceInfo{Host: "host-c", UserName: "myuser", Path: "C:/Users/myuser"}: &Policy{
+			RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(3)},
+		},
+		snapshot.SourceInfo{Host: "host-c", UserName: "myuser", Path: "C:/Users/myuser/dir1"}: &Policy{
+			RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(4)},
+		},
+		snapshot.SourceInfo{Host: "host-c", UserName: "myuser", Path: "C:/Users/myuser/dir2"}: &Policy{
+			RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(5)},
+		},
+		snapshot.SourceInfo{Host: "host-c", UserName: "myuser", Path: "C:/Users/myuser/dir2/a"}: &Policy{
+			RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(6)},
+		},
+		snapshot.SourceInfo{Host: "host-c", UserName: "myuser", Path: "C:/Users/myuser2"}: &Policy{
+			RetentionPolicy: RetentionPolicy{KeepDaily: intPtr(7)},
+		},
+	}
+
+	for si, pol := range setPols {
+		must(t, SetPolicy(ctx, env.Repository, si, pol))
+	}
+
+	cases := []struct {
+		si        snapshot.SourceInfo
+		wantPaths []string
+	}{
+		{snapshot.SourceInfo{Host: "host-a", UserName: "myuser", Path: "/tmp"}, []string{"."}},
+		{snapshot.SourceInfo{Host: "host-a", UserName: "myuser", Path: "/home/users/myuser"},
+			[]string{".", "./dir1", "./dir2", "./dir2/a"},
+		},
+		{snapshot.SourceInfo{Host: "host-a", UserName: "myuser", Path: "/home/users/myuser2"},
+			[]string{"."},
+		},
+		{snapshot.SourceInfo{Host: "host-a", UserName: "myuser", Path: "/home"},
+			[]string{
+				".",
+				"./users",
+				"./users/myuser",
+				"./users/myuser/dir1",
+				"./users/myuser/dir2",
+				"./users/myuser/dir2/a",
+				"./users/myuser2",
+			},
+		},
+		{snapshot.SourceInfo{Host: "host-a", UserName: "myuser", Path: "/"},
+			[]string{
+				".",
+				"./home",
+				"./home/users",
+				"./home/users/myuser",
+				"./home/users/myuser/dir1",
+				"./home/users/myuser/dir2",
+				"./home/users/myuser/dir2/a",
+				"./home/users/myuser2",
+			},
+		},
+
+		{snapshot.SourceInfo{Host: "host-b", UserName: "myuser", Path: "C:\\Temp"}, []string{"."}},
+		{snapshot.SourceInfo{Host: "host-b", UserName: "myuser", Path: "C:\\Users\\myuser"},
+			[]string{".", "./dir1", "./dir2", "./dir2/a"},
+		},
+		{snapshot.SourceInfo{Host: "host-b", UserName: "myuser", Path: "C:\\"},
+			[]string{
+				".",
+				"./Users",
+				"./Users/myuser",
+				"./Users/myuser/dir1",
+				"./Users/myuser/dir2",
+				"./Users/myuser/dir2/a",
+				"./Users/myuser2",
+			},
+		},
+	}
+
+	for _, tc := range cases {
+		tc := tc
+		t.Run(fmt.Sprintf("%v", tc.si), func(t *testing.T) {
+			res, err := applicablePoliciesForSource(ctx, env.Repository, tc.si)
+			if err != nil {
+				t.Fatalf("error in applicablePoliciesForSource(%v): %v", tc.si, err)
+			}
+
+			var relPaths []string
+			for k := range res {
+				relPaths = append(relPaths, k)
+			}
+
+			sort.Strings(relPaths)
+
+			if diff := cmp.Diff(relPaths, tc.wantPaths); diff != "" {
+				t.Errorf("invalid sub-policies %v", diff)
+			}
+		})
+
+	}
+}
+
 func must(t *testing.T, err error) {
 	t.Helper()
 

snapshot/policy/policy_tree_test.go

@@ -7,7 +7,7 @@ import (
 )
 
 var (
-	defaultPolicy = &Policy{
+	defPolicy = &Policy{
 		FilesPolicy: FilesPolicy{
 			IgnoreRules: []string{"default"},
 		},
@@ -84,7 +84,7 @@ func TestBuildTree(t *testing.T) {
 		".":              policyA,
 		"./foo":          policyB,
 		"./bar/baz/bleh": policyC,
-	}, defaultPolicy)
+	}, defPolicy)
 
 	dumpTree(n, "root")