Merge "Export jar-args.sh script to make"
diff --git a/Android.bp b/Android.bp
index 0c79bf5..d1b8f00 100644
--- a/Android.bp
+++ b/Android.bp
@@ -115,6 +115,7 @@
"cc/check.go",
"cc/coverage.go",
"cc/gen.go",
+ "cc/lto.go",
"cc/makevars.go",
"cc/prebuilt.go",
"cc/proto.go",
diff --git a/androidmk/cmd/androidmk/android.go b/androidmk/cmd/androidmk/android.go
index 194b2c9..4afb3b4 100644
--- a/androidmk/cmd/androidmk/android.go
+++ b/androidmk/cmd/androidmk/android.go
@@ -35,6 +35,7 @@
"LOCAL_SRC_FILES": srcFiles,
"LOCAL_SANITIZE": sanitize(""),
"LOCAL_SANITIZE_DIAG": sanitize("diag."),
+ "LOCAL_CFLAGS": cflags,
// composite functions
"LOCAL_MODULE_TAGS": includeVariableIf(bpVariable{"tags", bpparser.ListType}, not(valueDumpEquals("optional"))),
@@ -83,7 +84,6 @@
"LOCAL_SYSTEM_SHARED_LIBRARIES": "system_shared_libs",
"LOCAL_ASFLAGS": "asflags",
"LOCAL_CLANG_ASFLAGS": "clang_asflags",
- "LOCAL_CFLAGS": "cflags",
"LOCAL_CONLYFLAGS": "conlyflags",
"LOCAL_CPPFLAGS": "cppflags",
"LOCAL_REQUIRED_MODULES": "required",
@@ -532,6 +532,13 @@
return nil
}
+func cflags(ctx variableAssignmentContext) error {
+ // The Soong replacement for CFLAGS doesn't need the same extra escaped quotes that were present in Make
+ ctx.mkvalue = ctx.mkvalue.Clone()
+ ctx.mkvalue.ReplaceLiteral(`\"`, `"`)
+ return includeVariableNow(bpVariable{"cflags", bpparser.ListType}, ctx)
+}
+
// given a conditional, returns a function that will insert a variable assignment or not, based on the conditional
func includeVariableIf(bpVar bpVariable, conditional func(ctx variableAssignmentContext) bool) func(ctx variableAssignmentContext) error {
return func(ctx variableAssignmentContext) error {
diff --git a/androidmk/cmd/androidmk/androidmk_test.go b/androidmk/cmd/androidmk/androidmk_test.go
index 5fbc951..07d1c10 100644
--- a/androidmk/cmd/androidmk/androidmk_test.go
+++ b/androidmk/cmd/androidmk/androidmk_test.go
@@ -374,6 +374,25 @@
}
`,
},
+
+ {
+ desc: "Input containing escaped quotes",
+ in: `
+include $(CLEAR_VARS)
+LOCAL_MODULE:= libsensorservice
+LOCAL_CFLAGS:= -DLOG_TAG=\"-DDontEscapeMe\"
+LOCAL_SRC_FILES := \"EscapeMe.cc\"
+include $(BUILD_SHARED_LIBRARY)
+`,
+
+ expected: `
+cc_library_shared {
+ name: "libsensorservice",
+ cflags: ["-DLOG_TAG=\"-DDontEscapeMe\""],
+ srcs: ["\\\"EscapeMe.cc\\\""],
+}
+`,
+ },
}
func reformatBlueprint(input string) string {
diff --git a/androidmk/parser/make_strings.go b/androidmk/parser/make_strings.go
index 00d331b..142dc71 100644
--- a/androidmk/parser/make_strings.go
+++ b/androidmk/parser/make_strings.go
@@ -29,6 +29,11 @@
}
}
+func (ms *MakeString) Clone() (result *MakeString) {
+ clone := *ms
+ return &clone
+}
+
func (ms *MakeString) Pos() Pos {
return ms.StringPos
}
@@ -164,6 +169,12 @@
return s[len(s)-1] == uint8(ch)
}
+func (ms *MakeString) ReplaceLiteral(input string, output string) {
+ for i := range ms.Strings {
+ ms.Strings[i] = strings.Replace(ms.Strings[i], input, output, -1)
+ }
+}
+
func splitAnyN(s, sep string, n int) []string {
ret := []string{}
for n == -1 || n > 1 {
diff --git a/cc/cc.go b/cc/cc.go
index 983ffc0..ba06be2 100644
--- a/cc/cc.go
+++ b/cc/cc.go
@@ -51,6 +51,9 @@
ctx.BottomUp("coverage", coverageLinkingMutator).Parallel()
ctx.TopDown("vndk_deps", sabiDepsMutator)
+
+ ctx.TopDown("lto_deps", ltoDepsMutator)
+ ctx.BottomUp("lto", ltoMutator).Parallel()
})
pctx.Import("android/soong/cc/config")
@@ -295,6 +298,7 @@
coverage *coverage
sabi *sabi
vndkdep *vndkdep
+ lto *lto
androidMkSharedLibDeps []string
@@ -334,6 +338,9 @@
if c.vndkdep != nil {
c.AddProperties(c.vndkdep.props()...)
}
+ if c.lto != nil {
+ c.AddProperties(c.lto.props()...)
+ }
for _, feature := range c.features {
c.AddProperties(feature.props()...)
}
@@ -489,6 +496,7 @@
module.coverage = &coverage{}
module.sabi = &sabi{}
module.vndkdep = &vndkdep{}
+ module.lto = <o{}
return module
}
@@ -537,6 +545,9 @@
if c.coverage != nil {
flags = c.coverage.flags(ctx, flags)
}
+ if c.lto != nil {
+ flags = c.lto.flags(ctx, flags)
+ }
for _, feature := range c.features {
flags = feature.flags(ctx, flags)
}
@@ -620,6 +631,9 @@
if c.vndkdep != nil {
c.vndkdep.begin(ctx)
}
+ if c.lto != nil {
+ c.lto.begin(ctx)
+ }
for _, feature := range c.features {
feature.begin(ctx)
}
@@ -656,6 +670,9 @@
if c.vndkdep != nil {
deps = c.vndkdep.deps(ctx, deps)
}
+ if c.lto != nil {
+ deps = c.lto.deps(ctx, deps)
+ }
for _, feature := range c.features {
deps = feature.deps(ctx, deps)
}
@@ -1191,6 +1208,7 @@
&CoverageProperties{},
&SAbiProperties{},
&VndkProperties{},
+ <OProperties{},
)
android.InitDefaultsModule(module)
diff --git a/cc/config/arm64_device.go b/cc/config/arm64_device.go
index 025d3a5..0d22ed4 100644
--- a/cc/config/arm64_device.go
+++ b/cc/config/arm64_device.go
@@ -60,7 +60,7 @@
"-Wl,--build-id=md5",
"-Wl,--warn-shared-textrel",
"-Wl,--fatal-warnings",
- "-Wl,-maarch64linux",
+ "-Wl,-m,aarch64_elf64_le_vec",
"-Wl,--hash-style=gnu",
"-Wl,--fix-cortex-a53-843419",
"-fuse-ld=gold",
diff --git a/cc/config/arm_device.go b/cc/config/arm_device.go
index 38816aa..d50de2b 100644
--- a/cc/config/arm_device.go
+++ b/cc/config/arm_device.go
@@ -67,6 +67,7 @@
"-Wl,--icf=safe",
"-Wl,--hash-style=gnu",
"-Wl,--no-undefined-version",
+ "-Wl,-m,armelf",
}
armArmCflags = []string{
diff --git a/cc/config/global.go b/cc/config/global.go
index 56de351..82a44e6 100644
--- a/cc/config/global.go
+++ b/cc/config/global.go
@@ -16,6 +16,7 @@
import (
"fmt"
+ "runtime"
"strings"
"android/soong/android"
@@ -149,6 +150,11 @@
return ClangDefaultShortVersion, nil
})
pctx.StaticVariable("ClangAsanLibDir", "${ClangPath}/lib64/clang/${ClangShortVersion}/lib/linux")
+ if runtime.GOOS == "darwin" {
+ pctx.StaticVariable("LLVMGoldPlugin", "${ClangPath}/lib64/LLVMgold.dylib")
+ } else {
+ pctx.StaticVariable("LLVMGoldPlugin", "${ClangPath}/lib64/LLVMgold.so")
+ }
// These are tied to the version of LLVM directly in external/llvm, so they might trail the host prebuilts
// being used for the rest of the build process.
diff --git a/cc/lto.go b/cc/lto.go
new file mode 100644
index 0000000..f496772
--- /dev/null
+++ b/cc/lto.go
@@ -0,0 +1,117 @@
+// Copyright 2017 Google Inc. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package cc
+
+import (
+ "github.com/google/blueprint"
+
+ "android/soong/android"
+)
+
+// LTO (link-time optimization) allows the compiler to optimize and generate
+// code for the entire module at link time, rather than per-compilation
+// unit. LTO is required for Clang CFI and other whole-program optimization
+// techniques. LTO also allows cross-compilation unit optimizations that should
+// result in faster and smaller code, at the expense of additional compilation
+// time.
+//
+// To properly build a module with LTO, the module and all recursive static
+// dependencies should be compiled with -flto which directs the compiler to emit
+// bitcode rather than native object files. These bitcode files are then passed
+// by the linker to the LLVM plugin for compilation at link time. Static
+// dependencies not built as bitcode will still function correctly but cannot be
+// optimized at link time and may not be compatible with features that require
+// LTO, such as CFI.
+//
+// This file adds support to soong to automatically propogate LTO options to a
+// new variant of all static dependencies for each module with LTO enabled.
+
+type LTOProperties struct {
+ // Lto must violate capitialization style for acronyms so that it can be
+ // referred to in blueprint files as "lto"
+ Lto *bool `android:"arch_variant"`
+ LTODep bool `blueprint:"mutated"`
+}
+
+type lto struct {
+ Properties LTOProperties
+}
+
+func (lto *lto) props() []interface{} {
+ return []interface{}{<o.Properties}
+}
+
+func (lto *lto) begin(ctx BaseModuleContext) {
+}
+
+func (lto *lto) deps(ctx BaseModuleContext, deps Deps) Deps {
+ return deps
+}
+
+func (lto *lto) flags(ctx BaseModuleContext, flags Flags) Flags {
+ if Bool(lto.Properties.Lto) {
+ flags.CFlags = append(flags.CFlags, "-flto")
+ flags.LdFlags = append(flags.LdFlags, "-flto")
+ if ctx.Device() {
+ // Work around bug in Clang that doesn't pass correct emulated
+ // TLS option to target
+ flags.LdFlags = append(flags.LdFlags, "-Wl,-plugin-opt,-emulated-tls")
+ }
+ flags.ArFlags = append(flags.ArFlags, " --plugin ${config.LLVMGoldPlugin}")
+ }
+ return flags
+}
+
+// Can be called with a null receiver
+func (lto *lto) LTO() bool {
+ if lto == nil {
+ return false
+ }
+
+ return Bool(lto.Properties.Lto)
+}
+
+// Propagate lto requirements down from binaries
+func ltoDepsMutator(mctx android.TopDownMutatorContext) {
+ if c, ok := mctx.Module().(*Module); ok && c.lto.LTO() {
+ mctx.VisitDepsDepthFirst(func(m blueprint.Module) {
+ tag := mctx.OtherModuleDependencyTag(m)
+ switch tag {
+ case staticDepTag, staticExportDepTag, lateStaticDepTag, wholeStaticDepTag, objDepTag, reuseObjTag:
+ if cc, ok := m.(*Module); ok && cc.lto != nil {
+ cc.lto.Properties.LTODep = true
+ }
+ }
+ })
+ }
+}
+
+// Create lto variants for modules that need them
+func ltoMutator(mctx android.BottomUpMutatorContext) {
+ if c, ok := mctx.Module().(*Module); ok && c.lto != nil {
+ if c.lto.LTO() {
+ mctx.SetDependencyVariation("lto")
+ } else if c.lto.Properties.LTODep {
+ modules := mctx.CreateVariations("", "lto")
+ modules[0].(*Module).lto.Properties.Lto = boolPtr(false)
+ modules[1].(*Module).lto.Properties.Lto = boolPtr(true)
+ modules[0].(*Module).lto.Properties.LTODep = false
+ modules[1].(*Module).lto.Properties.LTODep = false
+ modules[1].(*Module).Properties.PreventInstall = true
+ modules[1].(*Module).Properties.HideFromMake = true
+ }
+ c.lto.Properties.LTODep = false
+ }
+}
diff --git a/cmd/soong_zip/soong_zip.go b/cmd/soong_zip/soong_zip.go
index 8407788..d634dda 100644
--- a/cmd/soong_zip/soong_zip.go
+++ b/cmd/soong_zip/soong_zip.go
@@ -646,15 +646,24 @@
}
func (z *zipWriter) writeDirectory(dir string) error {
- if dir != "" && !strings.HasSuffix(dir, "/") {
- dir = dir + "/"
+ // clean the input
+ cleanDir := filepath.Clean(dir)
+
+ // discover any uncreated directories in the path
+ zipDirs := []string{}
+ for cleanDir != "" && cleanDir != "." && !z.createdDirs[cleanDir] {
+
+ z.createdDirs[cleanDir] = true
+ // parent directories precede their children
+ zipDirs = append([]string{cleanDir}, zipDirs...)
+
+ cleanDir = filepath.Dir(cleanDir)
}
- for dir != "" && dir != "./" && !z.createdDirs[dir] {
- z.createdDirs[dir] = true
-
+ // make a directory entry for each uncreated directory
+ for _, cleanDir := range zipDirs {
dirHeader := &zip.FileHeader{
- Name: dir,
+ Name: cleanDir + "/",
}
dirHeader.SetMode(0700 | os.ModeDir)
dirHeader.SetModTime(z.time)
@@ -665,8 +674,6 @@
}
close(ze)
z.writeOps <- ze
-
- dir, _ = filepath.Split(dir)
}
return nil
diff --git a/finder/cmd/finder.go b/finder/cmd/finder.go
index 9da1660..70c1dc4 100644
--- a/finder/cmd/finder.go
+++ b/finder/cmd/finder.go
@@ -127,9 +127,13 @@
usage()
return errors.New("Param 'db' must be nonempty")
}
+
matches := []string{}
for i := 0; i < numIterations; i++ {
- matches = runFind(params, logger)
+ matches, err = runFind(params, logger)
+ if err != nil {
+ return err
+ }
}
findDuration := time.Since(startTime)
logger.Printf("Found these %v inodes in %v :\n", len(matches), findDuration)
@@ -142,8 +146,11 @@
return nil
}
-func runFind(params finder.CacheParams, logger *log.Logger) (paths []string) {
- service := finder.New(params, fs.OsFs, logger, dbPath)
+func runFind(params finder.CacheParams, logger *log.Logger) (paths []string, err error) {
+ service, err := finder.New(params, fs.OsFs, logger, dbPath)
+ if err != nil {
+ return []string{}, err
+ }
defer service.Shutdown()
- return service.FindAll()
+ return service.FindAll(), nil
}
diff --git a/finder/finder.go b/finder/finder.go
index ad85ee9..f15c8c1 100644
--- a/finder/finder.go
+++ b/finder/finder.go
@@ -150,6 +150,8 @@
// temporary state
threadPool *threadPool
mutex sync.Mutex
+ fsErrs []fsErr
+ errlock sync.Mutex
// non-temporary state
modifiedFlag int32
@@ -158,7 +160,7 @@
// New creates a new Finder for use
func New(cacheParams CacheParams, filesystem fs.FileSystem,
- logger Logger, dbPath string) *Finder {
+ logger Logger, dbPath string) (f *Finder, err error) {
numThreads := runtime.NumCPU() * 2
numDbLoadingThreads := numThreads
@@ -172,7 +174,7 @@
},
}
- finder := &Finder{
+ f = &Finder{
numDbLoadingThreads: numDbLoadingThreads,
numSearchingThreads: numSearchingThreads,
cacheMetadata: metadata,
@@ -183,10 +185,23 @@
DbPath: dbPath,
}
- finder.loadFromFilesystem()
+ f.loadFromFilesystem()
- finder.verbosef("Done parsing db\n")
- return finder
+ // check for any filesystem errors
+ err = f.getErr()
+ if err != nil {
+ return nil, err
+ }
+
+ // confirm that every path mentioned in the CacheConfig exists
+ for _, path := range cacheParams.RootDirs {
+ node := f.nodes.GetNode(filepath.Clean(path), false)
+ if node == nil || node.ModTime == 0 {
+ return nil, fmt.Errorf("%v does not exist\n", path)
+ }
+ }
+
+ return f, nil
}
// FindNamed searches for every cached file
@@ -338,10 +353,6 @@
f.startWithoutExternalCache()
}
- startTime := time.Now()
- f.verbosef("Waiting for pending requests to complete\n")
- f.threadPool.Wait()
- f.verbosef("Is idle after %v\n", time.Now().Sub(startTime))
f.threadPool = nil
}
@@ -598,6 +609,15 @@
p.receivedRequests.Wait()
}
+type fsErr struct {
+ path string
+ err error
+}
+
+func (e fsErr) String() string {
+ return e.path + ": " + e.err.Error()
+}
+
func (f *Finder) serializeCacheEntry(dirInfos []dirFullInfo) ([]byte, error) {
// group each dirFullInfo by its Device, to avoid having to repeat it in the output
dirsByDevice := map[uint64][]PersistedDirInfo{}
@@ -943,13 +963,17 @@
for i := range nodesToWalk {
f.listDirsAsync(nodesToWalk[i])
}
- f.verbosef("Loaded db and statted its contents in %v\n", time.Since(startTime))
+ f.verbosef("Loaded db and statted known dirs in %v\n", time.Since(startTime))
+ f.threadPool.Wait()
+ f.verbosef("Loaded db and statted all dirs in %v\n", time.Now().Sub(startTime))
+
return err
}
// startWithoutExternalCache starts scanning the filesystem according to the cache config
// startWithoutExternalCache should be called if startFromExternalCache is not applicable
func (f *Finder) startWithoutExternalCache() {
+ startTime := time.Now()
configDirs := f.cacheMetadata.Config.RootDirs
// clean paths
@@ -977,6 +1001,10 @@
f.verbosef("Starting find of %v\n", path)
f.startFind(path)
}
+
+ f.threadPool.Wait()
+
+ f.verbosef("Scanned filesystem (not using cache) in %v\n", time.Now().Sub(startTime))
}
// isInfoUpToDate tells whether <new> can confirm that results computed at <old> are still valid
@@ -1114,6 +1142,79 @@
f.verbosef("Wrote db in %v\n", time.Now().Sub(serializeDate))
return nil
+
+}
+
+// canIgnoreFsErr checks for certain classes of filesystem errors that are safe to ignore
+func (f *Finder) canIgnoreFsErr(err error) bool {
+ pathErr, isPathErr := err.(*os.PathError)
+ if !isPathErr {
+ // Don't recognize this error
+ return false
+ }
+ if pathErr.Err == os.ErrPermission {
+ // Permission errors are ignored:
+ // https://issuetracker.google.com/37553659
+ // https://github.com/google/kati/pull/116
+ return true
+ }
+ if pathErr.Err == os.ErrNotExist {
+ // If a directory doesn't exist, that generally means the cache is out-of-date
+ return true
+ }
+ // Don't recognize this error
+ return false
+}
+
+// onFsError should be called whenever a potentially fatal error is returned from a filesystem call
+func (f *Finder) onFsError(path string, err error) {
+ if !f.canIgnoreFsErr(err) {
+ // We could send the errors through a channel instead, although that would cause this call
+ // to block unless we preallocated a sufficient buffer or spawned a reader thread.
+ // Although it wouldn't be too complicated to spawn a reader thread, it's still slightly
+ // more convenient to use a lock. Only in an unusual situation should this code be
+ // invoked anyway.
+ f.errlock.Lock()
+ f.fsErrs = append(f.fsErrs, fsErr{path: path, err: err})
+ f.errlock.Unlock()
+ }
+}
+
+// discardErrsForPrunedPaths removes any errors for paths that are no longer included in the cache
+func (f *Finder) discardErrsForPrunedPaths() {
+ // This function could be somewhat inefficient due to being single-threaded,
+ // but the length of f.fsErrs should be approximately 0, so it shouldn't take long anyway.
+ relevantErrs := make([]fsErr, 0, len(f.fsErrs))
+ for _, fsErr := range f.fsErrs {
+ path := fsErr.path
+ node := f.nodes.GetNode(path, false)
+ if node != nil {
+ // The path in question wasn't pruned due to a failure to process a parent directory.
+ // So, the failure to process this path is important
+ relevantErrs = append(relevantErrs, fsErr)
+ }
+ }
+ f.fsErrs = relevantErrs
+}
+
+// getErr returns an error based on previous calls to onFsErr, if any
+func (f *Finder) getErr() error {
+ f.discardErrsForPrunedPaths()
+
+ numErrs := len(f.fsErrs)
+ if numErrs < 1 {
+ return nil
+ }
+
+ maxNumErrsToInclude := 10
+ message := ""
+ if numErrs > maxNumErrsToInclude {
+ message = fmt.Sprintf("finder encountered %v errors: %v...", numErrs, f.fsErrs[:maxNumErrsToInclude])
+ } else {
+ message = fmt.Sprintf("finder encountered %v errors: %v", numErrs, f.fsErrs)
+ }
+
+ return errors.New(message)
}
func (f *Finder) statDirAsync(dir *pathMap) {
@@ -1145,6 +1246,8 @@
var stats statResponse
if err != nil {
+ // possibly record this error
+ f.onFsError(path, err)
// in case of a failure to stat the directory, treat the directory as missing (modTime = 0)
return stats
}
@@ -1248,6 +1351,8 @@
children, err := f.filesystem.ReadDir(path)
if err != nil {
+ // possibly record this error
+ f.onFsError(path, err)
// if listing the contents of the directory fails (presumably due to
// permission denied), then treat the directory as empty
children = []os.FileInfo{}
diff --git a/finder/finder_test.go b/finder/finder_test.go
index 60e5eb2..15c3728 100644
--- a/finder/finder_test.go
+++ b/finder/finder_test.go
@@ -16,18 +16,17 @@
import (
"fmt"
+ "io/ioutil"
"log"
+ "os"
"path/filepath"
"reflect"
- "testing"
-
"sort"
-
- "io/ioutil"
+ "testing"
+ "time"
"android/soong/fs"
"runtime/debug"
- "time"
)
// some utils for tests to use
@@ -36,6 +35,14 @@
}
func newFinder(t *testing.T, filesystem *fs.MockFs, cacheParams CacheParams) *Finder {
+ f, err := newFinderAndErr(t, filesystem, cacheParams)
+ if err != nil {
+ fatal(t, err.Error())
+ }
+ return f
+}
+
+func newFinderAndErr(t *testing.T, filesystem *fs.MockFs, cacheParams CacheParams) (*Finder, error) {
cachePath := "/finder/finder-db"
cacheDir := filepath.Dir(cachePath)
filesystem.MkDirs(cacheDir)
@@ -44,16 +51,25 @@
}
logger := log.New(ioutil.Discard, "", 0)
- finder := New(cacheParams, filesystem, logger, cachePath)
- return finder
+ f, err := New(cacheParams, filesystem, logger, cachePath)
+ return f, err
}
func finderWithSameParams(t *testing.T, original *Finder) *Finder {
- return New(
+ f, err := finderAndErrorWithSameParams(t, original)
+ if err != nil {
+ fatal(t, err.Error())
+ }
+ return f
+}
+
+func finderAndErrorWithSameParams(t *testing.T, original *Finder) (*Finder, error) {
+ f, err := New(
original.cacheMetadata.Config.CacheParams,
original.filesystem,
original.logger,
original.DbPath)
+ return f, err
}
func write(t *testing.T, path string, content string, filesystem *fs.MockFs) {
@@ -61,7 +77,7 @@
filesystem.MkDirs(parent)
err := filesystem.WriteFile(path, []byte(content), 0777)
if err != nil {
- t.Fatal(err.Error())
+ fatal(t, err.Error())
}
}
@@ -72,21 +88,21 @@
func delete(t *testing.T, path string, filesystem *fs.MockFs) {
err := filesystem.Remove(path)
if err != nil {
- t.Fatal(err.Error())
+ fatal(t, err.Error())
}
}
func removeAll(t *testing.T, path string, filesystem *fs.MockFs) {
err := filesystem.RemoveAll(path)
if err != nil {
- t.Fatal(err.Error())
+ fatal(t, err.Error())
}
}
func move(t *testing.T, oldPath string, newPath string, filesystem *fs.MockFs) {
err := filesystem.Rename(oldPath, newPath)
if err != nil {
- t.Fatal(err.Error())
+ fatal(t, err.Error())
}
}
@@ -98,7 +114,7 @@
}
err = filesystem.Symlink(oldPath, newPath)
if err != nil {
- t.Fatal(err.Error())
+ fatal(t, err.Error())
}
}
func read(t *testing.T, path string, filesystem *fs.MockFs) string {
@@ -125,11 +141,20 @@
t.Fatal(err.Error())
}
}
+
+func setReadErr(t *testing.T, path string, readErr error, filesystem *fs.MockFs) {
+ err := filesystem.SetReadErr(path, readErr)
+ if err != nil {
+ t.Fatal(err.Error())
+ }
+}
+
func fatal(t *testing.T, message string) {
t.Error(message)
debug.PrintStack()
t.FailNow()
}
+
func assertSameResponse(t *testing.T, actual []string, expected []string) {
sort.Strings(actual)
sort.Strings(expected)
@@ -280,11 +305,11 @@
assertSameResponse(t, foundPaths, []string{createdPath})
}
-func TestNonexistentPath(t *testing.T) {
+func TestNonexistentDir(t *testing.T) {
filesystem := newFs()
create(t, "/tmp/findme.txt", filesystem)
- finder := newFinder(
+ _, err := newFinderAndErr(
t,
filesystem,
CacheParams{
@@ -292,11 +317,9 @@
IncludeFiles: []string{"findme.txt", "skipme.txt"},
},
)
- defer finder.Shutdown()
-
- foundPaths := finder.FindNamedAt("/tmp/IAlsoDontExist", "findme.txt")
-
- assertSameResponse(t, foundPaths, []string{})
+ if err == nil {
+ fatal(t, "Did not fail when given a nonexistent root directory")
+ }
}
func TestExcludeDirs(t *testing.T) {
@@ -392,7 +415,7 @@
t,
filesystem,
CacheParams{
- RootDirs: []string{"/IDoNotExist"},
+ RootDirs: []string{"/tmp/b"},
IncludeFiles: []string{"findme.txt"},
},
)
@@ -483,7 +506,7 @@
t,
filesystem,
CacheParams{
- RootDirs: []string{"/", "/a/b/c", "/a/b/c/d/e/f", "/a/b/c/d/e/f/g/h/i"},
+ RootDirs: []string{"/", "/tmp/a/b/c", "/tmp/a/b/c/d/e/f", "/tmp/a/b/c/d/e/f/g/h/i"},
IncludeFiles: []string{"findme.txt"},
},
)
@@ -1571,3 +1594,33 @@
// check results
assertSameResponse(t, foundPaths, []string{"/tmp/hi.txt"})
}
+
+func TestCacheEntryPathUnexpectedError(t *testing.T) {
+ // setup filesystem
+ filesystem := newFs()
+ create(t, "/tmp/a/hi.txt", filesystem)
+
+ // run the first finder
+ finder := newFinder(
+ t,
+ filesystem,
+ CacheParams{
+ RootDirs: []string{"/tmp"},
+ IncludeFiles: []string{"hi.txt"},
+ },
+ )
+ foundPaths := finder.FindAll()
+ filesystem.Clock.Tick()
+ finder.Shutdown()
+ // check results
+ assertSameResponse(t, foundPaths, []string{"/tmp/a/hi.txt"})
+
+ // make the directory not readable
+ setReadErr(t, "/tmp/a", os.ErrInvalid, filesystem)
+
+ // run the second finder
+ _, err := finderAndErrorWithSameParams(t, finder)
+ if err == nil {
+ fatal(t, "Failed to detect unexpected filesystem error")
+ }
+}
diff --git a/fs/fs.go b/fs/fs.go
index d8ef531..eff8ad0 100644
--- a/fs/fs.go
+++ b/fs/fs.go
@@ -159,7 +159,7 @@
permTime time.Time
sys interface{}
inodeNumber uint64
- readable bool
+ readErr error
}
func (m mockInode) ModTime() time.Time {
@@ -221,11 +221,11 @@
if err != nil {
return "", err
}
- if !parentNode.readable {
+ if parentNode.readErr != nil {
return "", &os.PathError{
Op: "read",
Path: path,
- Err: os.ErrPermission,
+ Err: parentNode.readErr,
}
}
@@ -240,11 +240,11 @@
}
}
- if !link.readable {
+ if link.readErr != nil {
return "", &os.PathError{
Op: "read",
Path: path,
- Err: os.ErrPermission,
+ Err: link.readErr,
}
}
@@ -277,11 +277,11 @@
Err: os.ErrNotExist,
}
}
- if !file.readable {
+ if file.readErr != nil {
return nil, &os.PathError{
Op: "open",
Path: fileName,
- Err: os.ErrPermission,
+ Err: file.readErr,
}
}
return file, nil
@@ -491,11 +491,11 @@
if err != nil {
return nil, err
}
- if !dir.readable {
+ if dir.readErr != nil {
return nil, &os.PathError{
Op: "read",
Path: path,
- Err: os.ErrPermission,
+ Err: dir.readErr,
}
}
// describe its contents
@@ -532,11 +532,11 @@
Err: os.ErrNotExist,
}
}
- if !sourceParentDir.readable {
+ if sourceParentDir.readErr != nil {
return &os.PathError{
Op: "move",
Path: sourcePath,
- Err: os.ErrPermission,
+ Err: sourceParentDir.readErr,
}
}
@@ -554,11 +554,11 @@
Err: os.ErrNotExist,
}
}
- if !destParentDir.readable {
+ if destParentDir.readErr != nil {
return &os.PathError{
Op: "move",
Path: destParentPath,
- Err: os.ErrPermission,
+ Err: destParentDir.readErr,
}
}
// check the source and dest themselves
@@ -648,11 +648,11 @@
Err: os.ErrNotExist,
}
}
- if !parentDir.readable {
+ if parentDir.readErr != nil {
return &os.PathError{
Op: "write",
Path: parentPath,
- Err: os.ErrPermission,
+ Err: parentDir.readErr,
}
}
@@ -662,11 +662,12 @@
parentDir.modTime = m.Clock.Time()
parentDir.files[baseName] = m.newFile()
} else {
- if !parentDir.files[baseName].readable {
+ readErr := parentDir.files[baseName].readErr
+ if readErr != nil {
return &os.PathError{
Op: "write",
Path: filePath,
- Err: os.ErrPermission,
+ Err: readErr,
}
}
}
@@ -681,7 +682,6 @@
newFile.inodeNumber = m.newInodeNumber()
newFile.modTime = m.Clock.Time()
newFile.permTime = newFile.modTime
- newFile.readable = true
return newFile
}
@@ -694,7 +694,6 @@
newDir.inodeNumber = m.newInodeNumber()
newDir.modTime = m.Clock.Time()
newDir.permTime = newDir.modTime
- newDir.readable = true
return newDir
}
@@ -705,7 +704,6 @@
newLink.inodeNumber = m.newInodeNumber()
newLink.modTime = m.Clock.Time()
newLink.permTime = newLink.modTime
- newLink.readable = true
return newLink
}
@@ -729,11 +727,11 @@
if err != nil {
return nil, err
}
- if !parent.readable {
+ if parent.readErr != nil {
return nil, &os.PathError{
Op: "stat",
Path: path,
- Err: os.ErrPermission,
+ Err: parent.readErr,
}
}
childDir, dirExists := parent.subdirs[leaf]
@@ -781,11 +779,11 @@
Err: os.ErrNotExist,
}
}
- if !parentDir.readable {
+ if parentDir.readErr != nil {
return &os.PathError{
Op: "remove",
Path: path,
- Err: os.ErrPermission,
+ Err: parentDir.readErr,
}
}
_, isDir := parentDir.subdirs[leaf]
@@ -822,11 +820,11 @@
newParentPath, leaf := pathSplit(newPath)
newParentDir, err := m.getDir(newParentPath, false)
- if !newParentDir.readable {
+ if newParentDir.readErr != nil {
return &os.PathError{
Op: "link",
Path: newPath,
- Err: os.ErrPermission,
+ Err: newParentDir.readErr,
}
}
if err != nil {
@@ -856,11 +854,11 @@
Err: os.ErrNotExist,
}
}
- if !parentDir.readable {
+ if parentDir.readErr != nil {
return &os.PathError{
Op: "removeAll",
Path: path,
- Err: os.ErrPermission,
+ Err: parentDir.readErr,
}
}
@@ -886,6 +884,14 @@
}
func (m *MockFs) SetReadable(path string, readable bool) error {
+ var readErr error
+ if !readable {
+ readErr = os.ErrPermission
+ }
+ return m.SetReadErr(path, readErr)
+}
+
+func (m *MockFs) SetReadErr(path string, readErr error) error {
path, err := m.resolve(path, false)
if err != nil {
return err
@@ -895,11 +901,11 @@
if err != nil {
return err
}
- if !parentDir.readable {
+ if parentDir.readErr != nil {
return &os.PathError{
Op: "chmod",
Path: parentPath,
- Err: os.ErrPermission,
+ Err: parentDir.readErr,
}
}
@@ -907,7 +913,7 @@
if err != nil {
return err
}
- inode.readable = readable
+ inode.readErr = readErr
inode.permTime = m.Clock.Time()
return nil
}