From e0418774889a35ef8099f8694b51a850c82589d0 Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Sat, 13 Jan 2024 18:58:23 +0100 Subject: lib/ignore: Refactor out result type (#9343) --- lib/fs/basicfs_watch.go | 4 +- lib/fs/basicfs_watch_test.go | 28 ++++---- lib/fs/filesystem.go | 7 +- lib/ignore/cache.go | 12 ++-- lib/ignore/cache_test.go | 4 +- lib/ignore/ignore.go | 83 ++++++---------------- lib/ignore/ignore_test.go | 3 +- lib/ignore/ignoreresult/ignoreresult.go | 75 +++++++++++++++++++ lib/ignore/ignoreresult/ignoreresult_foldcase.go | 11 +++ lib/ignore/ignoreresult/ignoreresult_nofoldcase.go | 11 +++ lib/model/folder_sendonly.go | 2 +- lib/model/folder_sendrecv.go | 2 +- 12 files changed, 155 insertions(+), 87 deletions(-) create mode 100644 lib/ignore/ignoreresult/ignoreresult.go create mode 100644 lib/ignore/ignoreresult/ignoreresult_foldcase.go create mode 100644 lib/ignore/ignoreresult/ignoreresult_nofoldcase.go diff --git a/lib/fs/basicfs_watch.go b/lib/fs/basicfs_watch.go index 83aea94e7..cc8349652 100644 --- a/lib/fs/basicfs_watch.go +++ b/lib/fs/basicfs_watch.go @@ -43,7 +43,7 @@ func (f *BasicFilesystem) Watch(name string, ignore Matcher, ctx context.Context if err != nil { return true } - return ignore.ShouldIgnore(rel) + return ignore.Match(rel).IsIgnored() } err = notify.WatchWithFilter(watchPath, backendChan, absShouldIgnore, eventMask) } else { @@ -94,7 +94,7 @@ func (f *BasicFilesystem) watchLoop(ctx context.Context, name string, roots []st return } - if ignore.ShouldIgnore(relPath) { + if ignore.Match(relPath).IsIgnored() { l.Debugln(f.Type(), f.URI(), "Watch: Ignoring", relPath) continue } diff --git a/lib/fs/basicfs_watch_test.go b/lib/fs/basicfs_watch_test.go index 244a4b3a9..290434c41 100644 --- a/lib/fs/basicfs_watch_test.go +++ b/lib/fs/basicfs_watch_test.go @@ -23,6 +23,7 @@ import ( "github.com/syncthing/notify" "github.com/syncthing/syncthing/lib/build" + "github.com/syncthing/syncthing/lib/ignore/ignoreresult" ) func TestMain(m *testing.M) { @@ -99,7 +100,7 @@ func TestWatchInclude(t *testing.T) { file := "file" ignored := "ignored" - testFs.MkdirAll(filepath.Join(name, ignored), 0777) + testFs.MkdirAll(filepath.Join(name, ignored), 0o777) included := filepath.Join(ignored, "included") testCase := func() { @@ -317,12 +318,12 @@ func TestWatchErrorLinuxInterpretation(t *testing.T) { t.Skip("testing of linux specific error codes") } - var errTooManyFiles = &os.PathError{ + errTooManyFiles := &os.PathError{ Op: "error while traversing", Path: "foo", Err: syscall.Errno(24), } - var errNoSpace = &os.PathError{ + errNoSpace := &os.PathError{ Op: "error while traversing", Path: "bar", Err: syscall.Errno(28), @@ -346,13 +347,13 @@ func TestWatchSymlinkedRoot(t *testing.T) { } name := "symlinkedRoot" - if err := testFs.MkdirAll(name, 0755); err != nil { + if err := testFs.MkdirAll(name, 0o755); err != nil { panic(fmt.Sprintf("Failed to create directory %s: %s", name, err)) } defer testFs.RemoveAll(name) root := filepath.Join(name, "root") - if err := testFs.MkdirAll(root, 0777); err != nil { + if err := testFs.MkdirAll(root, 0o777); err != nil { panic(err) } link := filepath.Join(name, "link") @@ -369,7 +370,7 @@ func TestWatchSymlinkedRoot(t *testing.T) { panic(err) } - if err := linkedFs.MkdirAll("foo", 0777); err != nil { + if err := linkedFs.MkdirAll("foo", 0o777); err != nil { panic(err) } @@ -507,7 +508,7 @@ func TestTruncateFileOnly(t *testing.T) { // path relative to folder root, also creates parent dirs if necessary func createTestFile(name string, file string) string { joined := filepath.Join(name, file) - if err := testFs.MkdirAll(filepath.Dir(joined), 0755); err != nil { + if err := testFs.MkdirAll(filepath.Dir(joined), 0o755); err != nil { panic(fmt.Sprintf("Failed to create parent directory for %s: %s", joined, err)) } handle, err := testFs.Create(joined) @@ -529,7 +530,7 @@ func renameTestFile(name string, old string, new string) { func modifyTestFile(name string, file string, content string) { joined := filepath.Join(testDirAbs, name, file) - err := os.WriteFile(joined, []byte(content), 0755) + err := os.WriteFile(joined, []byte(content), 0o755) if err != nil { panic(fmt.Sprintf("Failed to modify test file %s: %s", joined, err)) } @@ -540,7 +541,7 @@ func sleepMs(ms int) { } func testScenario(t *testing.T, name string, testCase func(), expectedEvents, allowedEvents []Event, fm fakeMatcher, ignorePerms bool) { - if err := testFs.MkdirAll(name, 0755); err != nil { + if err := testFs.MkdirAll(name, 0o755); err != nil { panic(fmt.Sprintf("Failed to create directory %s: %s", name, err)) } defer testFs.RemoveAll(name) @@ -567,7 +568,7 @@ func testScenario(t *testing.T, name string, testCase func(), expectedEvents, al } func testWatchOutput(t *testing.T, name string, in <-chan Event, expectedEvents, allowedEvents []Event, ctx context.Context, cancel context.CancelFunc) { - var expected = make(map[Event]struct{}) + expected := make(map[Event]struct{}) for _, ev := range expectedEvents { ev.Name = filepath.Join(name, ev.Name) expected[ev] = struct{}{} @@ -613,8 +614,11 @@ type fakeMatcher struct { skipIgnoredDirs bool } -func (fm fakeMatcher) ShouldIgnore(name string) bool { - return name != fm.include && name == fm.ignore +func (fm fakeMatcher) Match(name string) ignoreresult.R { + if name != fm.include && name == fm.ignore { + return ignoreresult.Ignored + } + return ignoreresult.NotIgnored } func (fm fakeMatcher) SkipIgnoredDirs() bool { diff --git a/lib/fs/filesystem.go b/lib/fs/filesystem.go index cf0b50cdb..f7232c9f7 100644 --- a/lib/fs/filesystem.go +++ b/lib/fs/filesystem.go @@ -16,6 +16,7 @@ import ( "strings" "time" + "github.com/syncthing/syncthing/lib/ignore/ignoreresult" "github.com/syncthing/syncthing/lib/protocol" ) @@ -127,14 +128,10 @@ type Usage struct { } type Matcher interface { - ShouldIgnore(name string) bool + Match(name string) ignoreresult.R SkipIgnoredDirs() bool } -type MatchResult interface { - IsIgnored() bool -} - type Event struct { Name string Type EventType diff --git a/lib/ignore/cache.go b/lib/ignore/cache.go index 5028f6575..e169c1b47 100644 --- a/lib/ignore/cache.go +++ b/lib/ignore/cache.go @@ -6,7 +6,11 @@ package ignore -import "time" +import ( + "time" + + "github.com/syncthing/syncthing/lib/ignore/ignoreresult" +) type nower interface { Now() time.Time @@ -20,7 +24,7 @@ type cache struct { } type cacheEntry struct { - result Result + result ignoreresult.R access int64 // Unix nanosecond count. Sufficient until the year 2262. } @@ -39,7 +43,7 @@ func (c *cache) clean(d time.Duration) { } } -func (c *cache) get(key string) (Result, bool) { +func (c *cache) get(key string) (ignoreresult.R, bool) { entry, ok := c.entries[key] if ok { entry.access = clock.Now().UnixNano() @@ -48,7 +52,7 @@ func (c *cache) get(key string) (Result, bool) { return entry.result, ok } -func (c *cache) set(key string, result Result) { +func (c *cache) set(key string, result ignoreresult.R) { c.entries[key] = cacheEntry{result, time.Now().UnixNano()} } diff --git a/lib/ignore/cache_test.go b/lib/ignore/cache_test.go index 68513203d..6aae99464 100644 --- a/lib/ignore/cache_test.go +++ b/lib/ignore/cache_test.go @@ -9,6 +9,8 @@ package ignore import ( "testing" "time" + + "github.com/syncthing/syncthing/lib/ignore/ignoreresult" ) func TestCache(t *testing.T) { @@ -28,7 +30,7 @@ func TestCache(t *testing.T) { // Set and check some items - c.set("true", resultInclude|resultDeletable) + c.set("true", ignoreresult.IgnoredDeletable) c.set("false", 0) res, ok = c.get("true") diff --git a/lib/ignore/ignore.go b/lib/ignore/ignore.go index 01960f3dd..db1392f1b 100644 --- a/lib/ignore/ignore.go +++ b/lib/ignore/ignore.go @@ -18,28 +18,13 @@ import ( "github.com/gobwas/glob" - "github.com/syncthing/syncthing/lib/build" "github.com/syncthing/syncthing/lib/fs" + "github.com/syncthing/syncthing/lib/ignore/ignoreresult" "github.com/syncthing/syncthing/lib/osutil" "github.com/syncthing/syncthing/lib/sha256" "github.com/syncthing/syncthing/lib/sync" ) -const ( - resultNotMatched Result = 0 - resultInclude Result = 1 << iota - resultDeletable = 1 << iota - resultFoldCase = 1 << iota -) - -var defaultResult Result = resultInclude - -func init() { - if build.IsDarwin || build.IsWindows { - defaultResult |= resultFoldCase - } -} - // A ParseError signifies an error with contents of an ignore file, // including I/O errors on included files. An I/O error on the root level // ignore file is not a ParseError. @@ -70,18 +55,18 @@ func parseError(err error) error { type Pattern struct { pattern string match glob.Glob - result Result + result ignoreresult.R } func (p Pattern) String() string { ret := p.pattern - if p.result&resultInclude != resultInclude { + if !p.result.IsIgnored() { ret = "!" + ret } - if p.result&resultFoldCase == resultFoldCase { + if p.result.IsCaseFolded() { ret = "(?i)" + ret } - if p.result&resultDeletable == resultDeletable { + if p.result.IsDeletable() { ret = "(?d)" + ret } return ret @@ -101,20 +86,6 @@ func (p Pattern) allowsSkippingIgnoredDirs() bool { return !strings.Contains(strings.TrimSuffix(p.pattern, "**"), "**") } -type Result uint8 - -func (r Result) IsIgnored() bool { - return r&resultInclude == resultInclude -} - -func (r Result) IsDeletable() bool { - return r.IsIgnored() && r&resultDeletable == resultDeletable -} - -func (r Result) IsCaseFolded() bool { - return r&resultFoldCase == resultFoldCase -} - // The ChangeDetector is responsible for determining if files have changed // on disk. It gets told to Remember() files (name and modtime) and will // then get asked if a file has been Seen() (i.e., Remember() has been @@ -253,16 +224,24 @@ func (m *Matcher) parseLocked(r io.Reader, file string) error { return err } -func (m *Matcher) Match(file string) (result Result) { - if file == "." { - return resultNotMatched +// Match matches the patterns plus temporary and internal files. +func (m *Matcher) Match(file string) (result ignoreresult.R) { + switch { + case fs.IsTemporary(file): + return ignoreresult.Ignored + + case fs.IsInternal(file): + return ignoreresult.Ignored + + case file == ".": + return ignoreresult.NotIgnored } m.mut.Lock() defer m.mut.Unlock() if len(m.patterns) == 0 { - return resultNotMatched + return ignoreresult.NotIgnored } if m.matches != nil { @@ -295,7 +274,7 @@ func (m *Matcher) Match(file string) (result Result) { } // Default to not matching. - return resultNotMatched + return ignoreresult.NotIgnored } // Lines return a list of the unprocessed lines in .stignore at last load @@ -348,22 +327,6 @@ func (m *Matcher) clean(d time.Duration) { } } -// ShouldIgnore returns true when a file is temporary, internal or ignored -func (m *Matcher) ShouldIgnore(filename string) bool { - switch { - case fs.IsTemporary(filename): - return true - - case fs.IsInternal(filename): - return true - - case m.Match(filename).IsIgnored(): - return true - } - - return false -} - func (m *Matcher) SkipIgnoredDirs() bool { m.mut.Lock() defer m.mut.Unlock() @@ -414,7 +377,7 @@ func loadParseIncludeFile(filesystem fs.Filesystem, file string, cd ChangeDetect // isNotExist is considered "ok" in a sense of that a folder doesn't have to act // upon it. This is because it is allowed for .stignore to not exist. However, // included ignore files are not allowed to be missing and these errors should be - // acted upon on. So we don't perserve the error chain here and manually set an + // acted upon on. So we don't preserve the error chain here and manually set an // error instead, if the file is missing. if fs.IsNotExist(err) { err = errors.New("file not found") @@ -431,7 +394,7 @@ func loadParseIncludeFile(filesystem fs.Filesystem, file string, cd ChangeDetect func parseLine(line string) ([]Pattern, error) { pattern := Pattern{ - result: defaultResult, + result: ignoreresult.Ignored, } // Allow prefixes to be specified in any order, but only once. @@ -441,14 +404,14 @@ func parseLine(line string) ([]Pattern, error) { if strings.HasPrefix(line, "!") && !seenPrefix[0] { seenPrefix[0] = true line = line[1:] - pattern.result ^= resultInclude + pattern.result = pattern.result.ToggleIgnored() } else if strings.HasPrefix(line, "(?i)") && !seenPrefix[1] { seenPrefix[1] = true - pattern.result |= resultFoldCase + pattern.result = pattern.result.WithFoldCase() line = line[4:] } else if strings.HasPrefix(line, "(?d)") && !seenPrefix[2] { seenPrefix[2] = true - pattern.result |= resultDeletable + pattern.result = pattern.result.WithDeletable() line = line[4:] } else { break diff --git a/lib/ignore/ignore_test.go b/lib/ignore/ignore_test.go index dbcdc32e7..ccdcd122e 100644 --- a/lib/ignore/ignore_test.go +++ b/lib/ignore/ignore_test.go @@ -17,6 +17,7 @@ import ( "github.com/syncthing/syncthing/lib/build" "github.com/syncthing/syncthing/lib/fs" + "github.com/syncthing/syncthing/lib/ignore/ignoreresult" "github.com/syncthing/syncthing/lib/osutil" "github.com/syncthing/syncthing/lib/rand" ) @@ -415,7 +416,7 @@ func TestCommentsAndBlankLines(t *testing.T) { } } -var result Result +var result ignoreresult.R func BenchmarkMatch(b *testing.B) { testFs := newTestFS() diff --git a/lib/ignore/ignoreresult/ignoreresult.go b/lib/ignore/ignoreresult/ignoreresult.go new file mode 100644 index 000000000..81a96b1da --- /dev/null +++ b/lib/ignore/ignoreresult/ignoreresult.go @@ -0,0 +1,75 @@ +// Copyright (C) 2024 The Syncthing Authors. +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this file, +// You can obtain one at https://mozilla.org/MPL/2.0/. + +// Package result provides the result type for ignore matching. This is a +// separate package in order to break import cycles. +package ignoreresult + +const ( + NotIgnored R = 0 + // `Ignored` is defined in platform specific files + IgnoredDeletable = Ignored | deletableBit +) + +const ( + // Private definitions of the bits that make up the result value + ignoreBit R = 1 << iota + deletableBit + foldCaseBit +) + +type R uint8 + +// IsIgnored returns true if the result is ignored. +func (r R) IsIgnored() bool { + return r&ignoreBit != 0 +} + +// IsDeletable returns true if the result is ignored and deletable. +func (r R) IsDeletable() bool { + return r.IsIgnored() && r&deletableBit != 0 +} + +// IsCaseFolded returns true if the result was a case-insensitive match. +func (r R) IsCaseFolded() bool { + return r&foldCaseBit != 0 +} + +// ToggleIgnored returns a copy of the result with the ignored bit toggled. +func (r R) ToggleIgnored() R { + return r ^ ignoreBit +} + +// WithDeletable returns a copy of the result with the deletable bit set. +func (r R) WithDeletable() R { + return r | deletableBit +} + +// WithFoldCase returns a copy of the result with the fold case bit set. +func (r R) WithFoldCase() R { + return r | foldCaseBit +} + +// String returns a human readable representation of the result flags. +func (r R) String() string { + var s string + if r&ignoreBit != 0 { + s += "i" + } else { + s += "-" + } + if r&deletableBit != 0 { + s += "d" + } else { + s += "-" + } + if r&foldCaseBit != 0 { + s += "f" + } else { + s += "-" + } + return s +} diff --git a/lib/ignore/ignoreresult/ignoreresult_foldcase.go b/lib/ignore/ignoreresult/ignoreresult_foldcase.go new file mode 100644 index 000000000..a77f13329 --- /dev/null +++ b/lib/ignore/ignoreresult/ignoreresult_foldcase.go @@ -0,0 +1,11 @@ +// Copyright (C) 2024 The Syncthing Authors. +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this file, +// You can obtain one at https://mozilla.org/MPL/2.0/. + +//go:build windows || darwin + +package ignoreresult + +const Ignored = ignoreBit | foldCaseBit diff --git a/lib/ignore/ignoreresult/ignoreresult_nofoldcase.go b/lib/ignore/ignoreresult/ignoreresult_nofoldcase.go new file mode 100644 index 000000000..55c7d23d5 --- /dev/null +++ b/lib/ignore/ignoreresult/ignoreresult_nofoldcase.go @@ -0,0 +1,11 @@ +// Copyright (C) 2024 The Syncthing Authors. +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this file, +// You can obtain one at https://mozilla.org/MPL/2.0/. + +//go:build !windows && !darwin + +package ignoreresult + +const Ignored = ignoreBit diff --git a/lib/model/folder_sendonly.go b/lib/model/folder_sendonly.go index f1e59c467..bf7dfc984 100644 --- a/lib/model/folder_sendonly.go +++ b/lib/model/folder_sendonly.go @@ -53,7 +53,7 @@ func (f *sendOnlyFolder) pull() (bool, error) { file := intf.(protocol.FileInfo) - if f.ignores.ShouldIgnore(intf.FileName()) { + if f.ignores.Match(intf.FileName()).IsIgnored() { file.SetIgnored() batch.Append(file) l.Debugln(f, "Handling ignored file", file) diff --git a/lib/model/folder_sendrecv.go b/lib/model/folder_sendrecv.go index 0e54cbfee..3356fc30c 100644 --- a/lib/model/folder_sendrecv.go +++ b/lib/model/folder_sendrecv.go @@ -343,7 +343,7 @@ func (f *sendReceiveFolder) processNeeded(snap *db.Snapshot, dbUpdateChan chan<- file := intf.(protocol.FileInfo) switch { - case f.ignores.ShouldIgnore(file.Name): + case f.ignores.Match(file.Name).IsIgnored(): file.SetIgnored() l.Debugln(f, "Handling ignored file", file) dbUpdateChan <- dbUpdateJob{file, dbUpdateInvalidate} -- cgit v1.2.3-54-g00ecf