diff --git a/basedirs/basedirs_test.go b/basedirs/basedirs_test.go index fb2394aa..00f2b30f 100644 --- a/basedirs/basedirs_test.go +++ b/basedirs/basedirs_test.go @@ -836,8 +836,7 @@ func TestBaseDirs(t *testing.T) { mainTable[0].DateNoSpace = time.Time{} mainTable[0].DateNoFiles = time.Time{} - So(len(mainTable), ShouldEqual, 7) - So(mainTable, ShouldResemble, []*Usage{ + mainTableExpectation := []*Usage{ { Name: "group1", GID: 1, UIDs: []uint32{101}, Owner: "Alan", BaseDir: projectA, UsageSize: twoGig + halfGig*2, QuotaSize: fiveGig, @@ -873,8 +872,25 @@ func TestBaseDirs(t *testing.T) { UsageSize: 60, QuotaSize: 500, UsageInodes: 1, QuotaInodes: 50, Mtime: expectedMtime, }, + } + + sort.Slice(mainTable, func(i, j int) bool { + return bytes.Compare( + idToByteSlice(mainTable[i].GID), + idToByteSlice(mainTable[j].GID), + ) != -1 + }) + + sort.Slice(mainTableExpectation, func(i, j int) bool { + return bytes.Compare( + idToByteSlice(mainTableExpectation[i].GID), + idToByteSlice(mainTableExpectation[j].GID), + ) != -1 }) + So(len(mainTable), ShouldEqual, 7) + So(mainTable, ShouldResemble, mainTableExpectation) + history, err := bdr.History(1, projectA) fixHistoryTimes(history) diff --git a/ch/ch.go b/ch/ch.go index 082a890c..e9883bd0 100644 --- a/ch/ch.go +++ b/ch/ch.go @@ -191,6 +191,10 @@ func groupName(gid int) (string, error) { } func (c *Ch) chmod(rule *Rule, path string, info fs.FileInfo) error { + if info.Mode()&fs.ModeSymlink == fs.ModeSymlink { + return nil + } + currentPerms := info.Mode() var desiredPerms fs.FileMode @@ -205,7 +209,7 @@ func (c *Ch) chmod(rule *Rule, path string, info fs.FileInfo) error { return nil } - if err := chmod(info, path, desiredPerms); err != nil { + if err := os.Chmod(path, desiredPerms); err != nil { return err } @@ -213,13 +217,3 @@ func (c *Ch) chmod(rule *Rule, path string, info fs.FileInfo) error { return nil } - -// chmod is like os.Chmod, but checks the given info to do nothing if this is a -// symlink. -func chmod(info fs.FileInfo, path string, mode fs.FileMode) error { - if info.Mode()&os.ModeSymlink == os.ModeSymlink { - return nil - } - - return os.Chmod(path, mode) -} diff --git a/ch/ch_test.go b/ch/ch_test.go index 49e3ae63..15ca5090 100644 --- a/ch/ch_test.go +++ b/ch/ch_test.go @@ -179,6 +179,32 @@ func TestCh(t *testing.T) { So(err.Error(), ShouldContainSubstring, "2 errors occurred") }) + Convey("Do on a symlink doesn't try to change permissions", func() { + mySymlink := filepath.Join(dir, "mySymlink") + + err := os.Symlink("non-existent-target", mySymlink) + So(err, ShouldBeNil) + + info, err := os.Lstat(mySymlink) + So(err, ShouldBeNil) + + err = ch.Do(mySymlink, info) + So(err, ShouldBeNil) + So(buff.String(), ShouldNotContainSubstring, `lvl=info msg="set permissions" path=`) + + createTestFile(t, filepath.Join(dir, "a"), otherGID, 0755) + + err = os.Remove(mySymlink) + So(err, ShouldBeNil) + + err = os.Symlink("a", mySymlink) + So(err, ShouldBeNil) + + err = ch.Do(mySymlink, info) + So(err, ShouldBeNil) + So(buff.String(), ShouldNotContainSubstring, `lvl=info msg="set permissions" path=`) + }) + Convey("chownGroup applies to symlinks themselves, not their targets", func() { path := filepath.Join(dir, "e") slink := filepath.Join(dir, "f") @@ -198,20 +224,6 @@ func TestCh(t *testing.T) { _, gid := getIDsFromFileInfo(info) So(gid, ShouldEqual, otherGID) - - Convey("chmod ignores symlinks but works on real files", func() { - err = chmod(info, slink, 0670) - So(err, ShouldBeNil) - - info, err = os.Lstat(path) - So(info.Mode().Perm(), ShouldEqual, fs.FileMode(0660)) - - err = chmod(info, path, 0670) - So(err, ShouldBeNil) - - info, err = os.Lstat(path) - So(info.Mode().Perm(), ShouldEqual, fs.FileMode(0670)) - }) }) }) })