Handle cache based on their contents instead of timestamps

5 files changed, 104 insertions, 67 deletions

diff --git a/eval.go b/eval.go
index 71d9691..52e6c13 100644
--- a/eval.go
+++ b/eval.go
@@ -3,28 +3,27 @@ package main
 import (
 	"bytes"
 	"fmt"
-	"os"
 	"strings"
 	"time"
 )
 
+const (
+	FILE_EXISTS       = 0
+	FILE_NOT_EXISTS   = 1
+	FILE_INCONSISTENT = 2 // Modified during kati is running.
+)
+
 type ReadMakefile struct {
 	Filename string
-	// We store the current time instead of the last modified time
-	// of the file. If we use the last modified time, we cannot
-	// notice the change of this file if the modification happens
-	// at the same second we read the file.
-	//
-	// -1 means the file did not exist and -2 means the timestamp
-	// was inconsistent.
-	Timestamp int64
+	Content  []byte
+	State    int32
 }
 
 type EvalResult struct {
 	vars     Vars
 	rules    []*Rule
 	ruleVars map[string]Vars
-	readMks  []ReadMakefile
+	readMks  []*ReadMakefile
 }
 
 type Evaluator struct {
@@ -37,7 +36,7 @@ type Evaluator struct {
 	currentScope Vars
 	avoidIO      bool
 	hasIO        bool
-	readMks      map[string]int64
+	readMks      map[string]*ReadMakefile
 
 	filename string
 	lineno   int
@@ -48,7 +47,7 @@ func newEvaluator(vars map[string]Var) *Evaluator {
 		outVars:     make(Vars),
 		vars:        vars,
 		outRuleVars: make(map[string]Vars),
-		readMks:     make(map[string]int64),
+		readMks:     make(map[string]*ReadMakefile),
 	}
 }
 
@@ -242,13 +241,13 @@ func (ev *Evaluator) LookupVarInCurrentScope(name string) Var {
 	return ev.vars.Lookup(name)
 }
 
-func (ev *Evaluator) evalIncludeFile(fname string, f *os.File) error {
+func (ev *Evaluator) evalIncludeFile(fname string, c []byte) error {
 	t := time.Now()
 	defer func() {
 		addStats("include", literal(fname), t)
 	}()
-	Log("Reading makefile %q", f)
-	mk, err := ParseMakefileFd(fname, f)
+	Log("Reading makefile %q", fname)
+	mk, err := ParseMakefile(c, fname)
 	if err != nil {
 		return err
 	}
@@ -262,21 +261,32 @@ func (ev *Evaluator) evalIncludeFile(fname string, f *os.File) error {
 	return nil
 }
 
-func (ev *Evaluator) updateMakefileTimestamp(fn string, ts int64) {
-	ts2, present := ev.readMks[fn]
+func (ev *Evaluator) updateReadMakefile(fn string, c []byte, st int32) {
+	rm, present := ev.readMks[fn]
 	if present {
-		if ts2 == -1 && ts != -1 {
-			Warn(ev.filename, ev.lineno, "%s was created after the previous read", fn)
-			ev.readMks[fn] = -2
-		} else if ts != -1 && ts == -1 {
-			Warn(ev.filename, ev.lineno, "%s was removed after the previous read", fn)
-			ev.readMks[fn] = -2
+		switch rm.State {
+		case FILE_EXISTS:
+			if st != FILE_EXISTS {
+				Warn(ev.filename, ev.lineno, "%s was removed after the previous read", fn)
+			} else if !bytes.Equal(c, rm.Content) {
+				Warn(ev.filename, ev.lineno, "%s was modified after the previous read", fn)
+				ev.readMks[fn].State = FILE_INCONSISTENT
+			}
+			return
+		case FILE_NOT_EXISTS:
+			if st != FILE_NOT_EXISTS {
+				Warn(ev.filename, ev.lineno, "%s was created after the previous read", fn)
+				ev.readMks[fn].State = FILE_INCONSISTENT
+			}
+		case FILE_INCONSISTENT:
+			return
 		}
-		// Just keep old value otherwise. If the content has
-		// been changed, we can detect the change by the
-		// timestamp check.
 	} else {
-		ev.readMks[fn] = ts
+		ev.readMks[fn] = &ReadMakefile{
+			Filename: fn,
+			Content:  c,
+			State:    st,
+		}
 	}
 }
 
@@ -296,19 +306,17 @@ func (ev *Evaluator) evalInclude(ast *IncludeAST) {
 	files := splitSpaces(buf.String())
 	buf.Reset()
 	for _, fn := range files {
-		now := time.Now().Unix()
-		f, err := os.Open(fn)
+		c, err := readFile(fn)
 		if err != nil {
 			if ast.op == "include" {
 				Error(ev.filename, ev.lineno, fmt.Sprintf("%v\nNOTE: kati does not support generating missing makefiles", err))
 			} else {
-				ev.updateMakefileTimestamp(fn, -1)
+				ev.updateReadMakefile(fn, nil, FILE_NOT_EXISTS)
 				continue
 			}
 		}
-		ev.updateMakefileTimestamp(fn, now)
-		defer f.Close()
-		err = ev.evalIncludeFile(fn, f)
+		ev.updateReadMakefile(fn, c, FILE_EXISTS)
+		err = ev.evalIncludeFile(fn, c)
 		if err != nil {
 			panic(err)
 		}
@@ -368,13 +376,10 @@ func (ev *Evaluator) eval(ast AST) {
 	ast.eval(ev)
 }
 
-func createTimestampArray(mp map[string]int64) []ReadMakefile {
-	var r []ReadMakefile
-	for k, v := range mp {
-		r = append(r, ReadMakefile{
-			Filename:  k,
-			Timestamp: v,
-		})
+func createReadMakefileArray(mp map[string]*ReadMakefile) []*ReadMakefile {
+	var r []*ReadMakefile
+	for _, v := range mp {
+		r = append(r, v)
 	}
 	return r
 }
@@ -399,6 +404,6 @@ func Eval(mk Makefile, vars Vars) (er *EvalResult, err error) {
 		vars:     ev.outVars,
 		rules:    ev.outRules,
 		ruleVars: ev.outRuleVars,
-		readMks:  createTimestampArray(ev.readMks),
+		readMks:  createReadMakefileArray(ev.readMks),
 	}, nil
 }
diff --git a/fileutil.go b/fileutil.go
index 80ad02e..2224d7d 100644
--- a/fileutil.go
+++ b/fileutil.go
@@ -1,6 +1,9 @@
 package main
 
-import "os"
+import (
+	"fmt"
+	"os"
+)
 
 func exists(filename string) bool {
 	f, err := os.Open(filename)
@@ -10,3 +13,23 @@ func exists(filename string) bool {
 	f.Close()
 	return true
 }
+
+func readFile(filename string) ([]byte, error) {
+	f, err := os.Open(filename)
+	if err != nil {
+		return nil, err
+	}
+	fi, err := f.Stat()
+	if err != nil {
+		return nil, err
+	}
+	buf := make([]byte, fi.Size())
+	n, err := f.Read(buf)
+	if err != nil {
+		return nil, err
+	}
+	if n != len(buf) {
+		return nil, fmt.Errorf("Unexpected file size: %d vs %d", n, len(buf))
+	}
+	return buf, nil
+}
diff --git a/main.go b/main.go
index d92456e..cfb516d 100644
--- a/main.go
+++ b/main.go
@@ -39,7 +39,7 @@ var (
 type DepGraph struct {
 	nodes    []*DepNode
 	vars     Vars
-	readMks  []ReadMakefile
+	readMks  []*ReadMakefile
 	isCached bool
 }
 
@@ -150,8 +150,11 @@ func getDepGraph(clvars []string, targets []string) *DepGraph {
 
 	bmk := getBootstrapMakefile(targets)
 
-	now := time.Now().Unix()
-	mk, err := ParseMakefile(makefile)
+	content, err := readFile(makefile)
+	if err != nil {
+		panic(err)
+	}
+	mk, err := ParseMakefile(content, makefile)
 	if err != nil {
 		panic(err)
 	}
@@ -207,11 +210,12 @@ func getDepGraph(clvars []string, targets []string) *DepGraph {
 		panic(err2)
 	}
 	LogStats("dep build time: %q", time.Now().Sub(startTime))
-	var readMks []ReadMakefile
+	var readMks []*ReadMakefile
 	// Always put the root Makefile as the first element.
-	readMks = append(readMks, ReadMakefile{
-		Filename:  makefile,
-		Timestamp: now,
+	readMks = append(readMks, &ReadMakefile{
+		Filename: makefile,
+		Content:  content,
+		State:    FILE_EXISTS,
 	})
 	readMks = append(readMks, er.readMks...)
 	return &DepGraph{
diff --git a/parser.go b/parser.go
index 489f77f..b6ed84c 100644
--- a/parser.go
+++ b/parser.go
@@ -8,7 +8,6 @@ package main
 import (
 	"bufio"
 	"bytes"
-	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -574,6 +573,7 @@ func ParseMakefileFd(filename string, f *os.File) (Makefile, error) {
 	return parser.parse()
 }
 
+/*
 func ParseMakefile(filename string) (Makefile, error) {
 	Log("ParseMakefile %q", filename)
 	f, err := os.Open(filename)
@@ -594,6 +594,7 @@ func ParseDefaultMakefile() (Makefile, string, error) {
 	}
 	return Makefile{}, "", errors.New("no targets specified and no makefile found.")
 }
+*/
 
 func GetDefaultMakefile() string {
 	candidates := []string{"GNUmakefile", "makefile", "Makefile"}
@@ -621,3 +622,9 @@ func ParseMakefileString(s string, name string, lineno int) (Makefile, error) {
 func ParseMakefileBytes(s []byte, name string, lineno int) (Makefile, error) {
 	return parseMakefileReader(bytes.NewReader(s), name, lineno)
 }
+
+func ParseMakefile(s []byte, filename string) (Makefile, error) {
+	Log("ParseMakefile %q", filename)
+	parser := newParser(bytes.NewReader(s), filename)
+	return parser.parse()
+}
diff --git a/serialize.go b/serialize.go
index f8f509d..4c5c8ef 100644
--- a/serialize.go
+++ b/serialize.go
@@ -94,7 +94,7 @@ type SerializableGraph struct {
 	Tsvs    []SerializableTargetSpecificVar
 	Targets []string
 	Roots   []string
-	ReadMks []ReadMakefile
+	ReadMks []*ReadMakefile
 }
 
 func encGob(v interface{}) string {
@@ -265,7 +265,7 @@ func DumpDepGraphCache(g *DepGraph, roots []string) {
 	cacheFile := GetCacheFilename(g.readMks[0].Filename, roots)
 	for _, mk := range g.readMks {
 		// Inconsistent, do not dump this result.
-		if mk.Timestamp == -2 {
+		if mk.State == 2 {
 			if exists(cacheFile) {
 				os.Remove(cacheFile)
 			}
@@ -568,22 +568,20 @@ func LoadDepGraphCache(makefile string, roots []string) *DepGraph {
 
 	g := LoadDepGraph(filename)
 	for _, mk := range g.readMks {
-		// TODO: A hack for Android build. Maybe we should
-		// keep their contents instead of the timestamp.
-		if mk.Filename == "out/target/product/generic/clean_steps.mk" ||
-			mk.Filename == "out/target/common/obj/previous_aidl_config.mk" ||
-			mk.Filename == "out/target/product/generic/previous_build_config.mk" {
-			continue
-		}
-
-		ts := getTimestamp(mk.Filename)
-		if mk.Timestamp >= 0 && ts < 0 {
-			LogAlways("Cache expired: %s", mk.Filename)
-			return nil
+		if mk.State != FILE_EXISTS && mk.State != FILE_NOT_EXISTS {
+			panic(fmt.Sprintf("Internal error: broken state: %d", mk.State))
 		}
-		if mk.Timestamp <= ts {
-			LogAlways("Cache expired: %s", mk.Filename)
-			return nil
+		if mk.State == FILE_NOT_EXISTS {
+			if exists(mk.Filename) {
+				LogAlways("Cache expired: %s", mk.Filename)
+				return nil
+			}
+		} else {
+			c, err := readFile(mk.Filename)
+			if err != nil || !bytes.Equal(c, mk.Content) {
+				LogAlways("Cache expired: %s", mk.Filename)
+				return nil
+			}
 		}
 	}
 	g.isCached = true