From 54cb95a53f4e41311c5307da5c5911e31330ce2c Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Fri, 23 Feb 2018 11:09:18 -0800 Subject: [PATCH] Fix glob cache conflict when excludes=nil and excludes=[]string{} Performing the same glob twice, once with excludes=nil and once with excludes=[]string{} would hit the same entry in the glob cache (since the glob filename would be the same), but fail the verifyGlob check because DeepEqual considers []string(nil) and []string{} to be different. Use a manual array check instead. Test: glob_test.go Change-Id: If0d4fe80163a871077b7276e1b4a3e888a4a4898 --- Blueprints | 1 + glob.go | 9 +++++++-- glob_test.go | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 glob_test.go diff --git a/Blueprints b/Blueprints index de18298..c892b46 100644 --- a/Blueprints +++ b/Blueprints @@ -23,6 +23,7 @@ bootstrap_go_package { ], testSrcs: [ "context_test.go", + "glob_test.go", "ninja_strings_test.go", "ninja_writer_test.go", "splice_modules_test.go", diff --git a/glob.go b/glob.go index f3eb1b1..4553f69 100644 --- a/glob.go +++ b/glob.go @@ -17,7 +17,6 @@ package blueprint import ( "crypto/md5" "fmt" - "reflect" "sort" "strings" ) @@ -34,9 +33,15 @@ func verifyGlob(fileName, pattern string, excludes []string, g GlobPath) { if pattern != g.Pattern { panic(fmt.Errorf("Mismatched patterns %q and %q for glob file %q", pattern, g.Pattern, fileName)) } - if !reflect.DeepEqual(g.Excludes, excludes) { + if len(excludes) != len(g.Excludes) { panic(fmt.Errorf("Mismatched excludes %v and %v for glob file %q", excludes, g.Excludes, fileName)) } + + for i := range excludes { + if g.Excludes[i] != excludes[i] { + panic(fmt.Errorf("Mismatched excludes %v and %v for glob file %q", excludes, g.Excludes, fileName)) + } + } } func (c *Context) glob(pattern string, excludes []string) ([]string, error) { diff --git a/glob_test.go b/glob_test.go new file mode 100644 index 0000000..3fff5a8 --- /dev/null +++ b/glob_test.go @@ -0,0 +1,55 @@ +// Copyright 2018 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 blueprint + +import "testing" + +func TestGlobCache(t *testing.T) { + ctx := NewContext() + ctx.MockFileSystem(map[string][]byte{ + "Blueprints": nil, + "a/a": nil, + "a/b": nil, + }) + + // Test a simple glob with no excludes + matches, err := ctx.glob("a/*", nil) + if err != nil { + t.Error("unexpected error", err) + } + if len(matches) != 2 || matches[0] != "a/a" || matches[1] != "a/b" { + t.Error(`expected ["a/a", "a/b"], got`, matches) + } + + // Test the same glob with an empty excludes array to make sure + // excludes=nil does not conflict with excludes=[]string{} in the + // cache. + matches, err = ctx.glob("a/*", []string{}) + if err != nil { + t.Error("unexpected error", err) + } + if len(matches) != 2 || matches[0] != "a/a" || matches[1] != "a/b" { + t.Error(`expected ["a/a", "a/b"], got`, matches) + } + + // Test the same glob with a different excludes array + matches, err = ctx.glob("a/*", []string{"a/b"}) + if err != nil { + t.Error("unexpected error", err) + } + if len(matches) != 1 || matches[0] != "a/a" { + t.Error(`expected ["a/a"], got`, matches) + } +}