From fe4bc36f87b9d4f529bfd486b89176f3f3045323 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Wed, 12 Sep 2018 10:02:13 -0700 Subject: [PATCH 1/2] Allow '$' in some paths The icu resource directories contain filenames that have '$' characters. Allow paths returned by the Glob functions to contain '$', on the assumption that real paths on disk are unlikely to contain strings that are valid ninja variables. Fix the Build rules to escape any paths that are passed as Path arguments. Fix the resource rules to manually escape the paths that are passed as strings. Test: m checkbuild Change-Id: Ie631bc6d96259e592adb280491a365c0df7ed0e2 --- android/module.go | 8 ++++++++ android/package_ctx.go | 16 +++++++++++++--- android/paths.go | 25 ++++++++++++------------- java/builder.go | 3 ++- 4 files changed, 35 insertions(+), 17 deletions(-) diff --git a/android/module.go b/android/module.go index 77765f1b1..4d9ddd4b1 100644 --- a/android/module.go +++ b/android/module.go @@ -23,6 +23,7 @@ import ( "github.com/google/blueprint" "github.com/google/blueprint/pathtools" + "github.com/google/blueprint/proptools" ) var ( @@ -844,6 +845,13 @@ func convertBuildParams(params BuildParams) blueprint.BuildParams { bparams.Implicits = append(bparams.Implicits, params.Implicit.String()) } + bparams.Outputs = proptools.NinjaEscape(bparams.Outputs) + bparams.ImplicitOutputs = proptools.NinjaEscape(bparams.ImplicitOutputs) + bparams.Inputs = proptools.NinjaEscape(bparams.Inputs) + bparams.Implicits = proptools.NinjaEscape(bparams.Implicits) + bparams.OrderOnly = proptools.NinjaEscape(bparams.OrderOnly) + bparams.Depfile = proptools.NinjaEscape([]string{bparams.Depfile})[0] + return bparams } diff --git a/android/package_ctx.go b/android/package_ctx.go index e228bba56..00b99ff62 100644 --- a/android/package_ctx.go +++ b/android/package_ctx.go @@ -124,7 +124,11 @@ func (p PackageContext) RuleFunc(name string, // package-scoped variable's initialization. func (p PackageContext) SourcePathVariable(name, path string) blueprint.Variable { return p.VariableFunc(name, func(ctx PackageVarContext) string { - return safePathForSource(ctx, path).String() + p, err := safePathForSource(ctx, path) + if err != nil { + ctx.Errorf("%s", err.Error()) + } + return p.String() }) } @@ -136,7 +140,10 @@ func (p PackageContext) SourcePathsVariable(name, separator string, paths ...str return p.VariableFunc(name, func(ctx PackageVarContext) string { var ret []string for _, path := range paths { - p := safePathForSource(ctx, path) + p, err := safePathForSource(ctx, path) + if err != nil { + ctx.Errorf("%s", err.Error()) + } ret = append(ret, p.String()) } return strings.Join(ret, separator) @@ -150,7 +157,10 @@ func (p PackageContext) SourcePathsVariable(name, separator string, paths ...str // as part of a package-scoped variable's initialization. func (p PackageContext) SourcePathVariableWithEnvOverride(name, path, env string) blueprint.Variable { return p.VariableFunc(name, func(ctx PackageVarContext) string { - p := safePathForSource(ctx, path) + p, err := safePathForSource(ctx, path) + if err != nil { + ctx.Errorf("%s", err.Error()) + } return ctx.Config().GetenvWithDefault(env, p.String()) }) } diff --git a/android/paths.go b/android/paths.go index 57ebae2a3..91abeba5f 100644 --- a/android/paths.go +++ b/android/paths.go @@ -230,6 +230,8 @@ func PathsForModuleSrc(ctx ModuleContext, paths []string) Paths { // pathsForModuleSrcFromFullPath returns Paths rooted from the module's local // source directory, but strip the local source directory from the beginning of // each string. If incDirs is false, strip paths with a trailing '/' from the list. +// It intended for use in globs that only list files that exist, so it allows '$' in +// filenames. func pathsForModuleSrcFromFullPath(ctx ModuleContext, paths []string, incDirs bool) Paths { prefix := filepath.Join(ctx.Config().srcDir, ctx.ModuleDir()) + "/" if prefix == "./" { @@ -246,7 +248,7 @@ func pathsForModuleSrcFromFullPath(ctx ModuleContext, paths []string, incDirs bo continue } - srcPath, err := pathForSource(ctx, ctx.ModuleDir(), path[len(prefix):]) + srcPath, err := safePathForSource(ctx, ctx.ModuleDir(), path[len(prefix):]) if err != nil { reportPathError(ctx, err) continue @@ -494,29 +496,26 @@ func (p SourcePath) withRel(rel string) SourcePath { // safePathForSource is for paths that we expect are safe -- only for use by go // code that is embedding ninja variables in paths -func safePathForSource(ctx PathContext, path string) SourcePath { - p, err := validateSafePath(path) - if err != nil { - reportPathError(ctx, err) - } +func safePathForSource(ctx PathContext, pathComponents ...string) (SourcePath, error) { + p, err := validateSafePath(pathComponents...) ret := SourcePath{basePath{p, ctx.Config(), ""}} + if err != nil { + return ret, err + } abs, err := filepath.Abs(ret.String()) if err != nil { - reportPathError(ctx, err) - return ret + return ret, err } buildroot, err := filepath.Abs(ctx.Config().buildDir) if err != nil { - reportPathError(ctx, err) - return ret + return ret, err } if strings.HasPrefix(abs, buildroot) { - reportPathErrorf(ctx, "source path %s is in output", abs) - return ret + return ret, fmt.Errorf("source path %s is in output", abs) } - return ret + return ret, err } // pathForSource creates a SourcePath from pathComponents, but does not check that it exists. diff --git a/java/builder.go b/java/builder.go index bb1590aec..48b5a7b6c 100644 --- a/java/builder.go +++ b/java/builder.go @@ -24,6 +24,7 @@ import ( "strings" "github.com/google/blueprint" + "github.com/google/blueprint/proptools" "android/soong/android" ) @@ -320,7 +321,7 @@ func TransformResourcesToJar(ctx android.ModuleContext, outputFile android.Writa Output: outputFile, Implicits: deps, Args: map[string]string{ - "jarArgs": strings.Join(jarArgs, " "), + "jarArgs": strings.Join(proptools.NinjaEscape(jarArgs), " "), }, }) } From cedd4768f58eab69cfbce8c2dc599d83a0c7dd26 Mon Sep 17 00:00:00 2001 From: Colin Cross Date: Thu, 13 Sep 2018 11:26:19 -0700 Subject: [PATCH 2/2] Allow exclude_java_resources to affect java_resource_dirs Allow excluding files from directory globbed by java_resource_dirs. Test: java_test.go Change-Id: I9922842248be1a386ab111a5187608438638ffb1 --- java/java.go | 5 ++-- java/java_resources.go | 4 ++- java/java_test.go | 60 +++++++++++++++++------------------------- 3 files changed, 30 insertions(+), 39 deletions(-) diff --git a/java/java.go b/java/java.go index b60f9c739..4bf5880bc 100644 --- a/java/java.go +++ b/java/java.go @@ -75,7 +75,7 @@ type CompilerProperties struct { // list of files to use as Java resources Java_resources []string `android:"arch_variant"` - // list of files that should be excluded from java_resources + // list of files that should be excluded from java_resources and java_resource_dirs Exclude_java_resources []string `android:"arch_variant"` // don't build against the default libraries (bootclasspath, legacy-test, core-junit, @@ -1119,7 +1119,8 @@ func (j *Module) compile(ctx android.ModuleContext, extraSrcJars ...android.Path } } - dirArgs, dirDeps := ResourceDirsToJarArgs(ctx, j.properties.Java_resource_dirs, j.properties.Exclude_java_resource_dirs) + dirArgs, dirDeps := ResourceDirsToJarArgs(ctx, j.properties.Java_resource_dirs, + j.properties.Exclude_java_resource_dirs, j.properties.Exclude_java_resources) fileArgs, fileDeps := ResourceFilesToJarArgs(ctx, j.properties.Java_resources, j.properties.Exclude_java_resources) var resArgs []string diff --git a/java/java_resources.go b/java/java_resources.go index e02709dc0..fdc159063 100644 --- a/java/java_resources.go +++ b/java/java_resources.go @@ -32,7 +32,7 @@ var resourceExcludes = []string{ } func ResourceDirsToJarArgs(ctx android.ModuleContext, - resourceDirs, excludeResourceDirs []string) (args []string, deps android.Paths) { + resourceDirs, excludeResourceDirs, excludeResourceFiles []string) (args []string, deps android.Paths) { var excludeDirs []string var excludeFiles []string @@ -44,6 +44,8 @@ func ResourceDirsToJarArgs(ctx android.ModuleContext, } } + excludeFiles = append(excludeFiles, ctx.ExpandSources(excludeResourceFiles, nil).Strings()...) + excludeFiles = append(excludeFiles, resourceExcludes...) for _, resourceDir := range resourceDirs { diff --git a/java/java_test.go b/java/java_test.go index 3ace528da..82accd5a5 100644 --- a/java/java_test.go +++ b/java/java_test.go @@ -702,6 +702,30 @@ func TestResources(t *testing.T) { prop: `java_resource_dirs: ["java-res/*"], exclude_java_resource_dirs: ["java-res/b"]`, args: "-C java-res/a -f java-res/a/a", }, + { + // Test wildcards in java_resources + name: "wildcard files", + prop: `java_resources: ["java-res/**/*"]`, + args: "-C . -f java-res/a/a -f java-res/b/b", + }, + { + // Test exclude_java_resources with java_resources + name: "wildcard files with exclude", + prop: `java_resources: ["java-res/**/*"], exclude_java_resources: ["java-res/b/*"]`, + args: "-C . -f java-res/a/a", + }, + { + // Test exclude_java_resources with java_resource_dirs + name: "resource dirs with exclude files", + prop: `java_resource_dirs: ["java-res"], exclude_java_resources: ["java-res/b/b"]`, + args: "-C java-res -f java-res/a/a", + }, + { + // Test exclude_java_resource_dirs with java_resource_dirs + name: "resource dirs with exclude files", + prop: `java_resource_dirs: ["java-res", "java-res2"], exclude_java_resource_dirs: ["java-res2"]`, + args: "-C java-res -f java-res/a/a -f java-res/b/b", + }, } for _, test := range table { @@ -734,42 +758,6 @@ func TestResources(t *testing.T) { } } -func TestExcludeResources(t *testing.T) { - ctx := testJava(t, ` - java_library { - name: "foo", - srcs: ["a.java"], - java_resource_dirs: ["java-res", "java-res2"], - exclude_java_resource_dirs: ["java-res2"], - } - - java_library { - name: "bar", - srcs: ["a.java"], - java_resources: ["java-res/*/*"], - exclude_java_resources: ["java-res/b/*"], - } - `) - - fooRes := ctx.ModuleForTests("foo", "android_common").Output("res/foo.jar") - - expected := "-C java-res -f java-res/a/a -f java-res/b/b" - if fooRes.Args["jarArgs"] != expected { - t.Errorf("foo resource jar args %q is not %q", - fooRes.Args["jarArgs"], expected) - - } - - barRes := ctx.ModuleForTests("bar", "android_common").Output("res/bar.jar") - - expected = "-C . -f java-res/a/a" - if barRes.Args["jarArgs"] != expected { - t.Errorf("bar resource jar args %q is not %q", - barRes.Args["jarArgs"], expected) - - } -} - func TestGeneratedSources(t *testing.T) { ctx := testJava(t, ` java_library {