From e3487c88482c96b31c8bd5ff4f14d3bc0e70b784 Mon Sep 17 00:00:00 2001 From: "Lukacs T. Berki" Date: Mon, 2 May 2022 10:13:19 +0200 Subject: [PATCH] Add a test for correctness of C++ compilation. This required the following: - Adding Platform_base_sdk_extension_version to default soong.variables - Teaching the symlink tree creation code to understand symlinks - Making finder.go follow symlinks when requested Adding yet another knob is unfortunate, but I can't allow that unconditionally because the Android code base contains a number of symlinks giving rise to infinite directory trees because they point back to their parent and this seemed preferable to adding complicated logic like "follow symlink but if only its fully resolved version does not point under the source tree". I could be convinced about the latter, though. Test: Presubmits. Change-Id: I453f6b7e5334771f5832c700db00f9d24ed1d82f --- android/variable.go | 13 +++++---- bp2build/symlink_forest.go | 32 ++++++++++++++++++--- finder/finder.go | 15 ++++++++-- finder/finder_test.go | 2 ++ tests/bp2build_bazel_test.sh | 54 ++++++++++++++++++++++++++++++++++++ tests/lib.sh | 4 +++ ui/build/finder.go | 1 + 7 files changed, 108 insertions(+), 13 deletions(-) diff --git a/android/variable.go b/android/variable.go index 90cb6ff15..9478c0c9e 100644 --- a/android/variable.go +++ b/android/variable.go @@ -463,12 +463,13 @@ func (v *productVariables) SetDefaultConfig() { *v = productVariables{ BuildNumberFile: stringPtr("build_number.txt"), - Platform_version_name: stringPtr("S"), - Platform_sdk_version: intPtr(30), - Platform_sdk_codename: stringPtr("S"), - Platform_sdk_final: boolPtr(false), - Platform_version_active_codenames: []string{"S"}, - Platform_vndk_version: stringPtr("S"), + Platform_version_name: stringPtr("S"), + Platform_base_sdk_extension_version: intPtr(30), + Platform_sdk_version: intPtr(30), + Platform_sdk_codename: stringPtr("S"), + Platform_sdk_final: boolPtr(false), + Platform_version_active_codenames: []string{"S"}, + Platform_vndk_version: stringPtr("S"), HostArch: stringPtr("x86_64"), HostSecondaryArch: stringPtr("x86"), diff --git a/bp2build/symlink_forest.go b/bp2build/symlink_forest.go index 15a63356e..7d4819110 100644 --- a/bp2build/symlink_forest.go +++ b/bp2build/symlink_forest.go @@ -90,6 +90,20 @@ func symlinkIntoForest(topdir, dst, src string) { } } +func isDir(path string, fi os.FileInfo) bool { + if (fi.Mode() & os.ModeSymlink) != os.ModeSymlink { + return fi.IsDir() + } + + fi2, err := os.Stat(path) + if err != nil { + fmt.Fprintf(os.Stderr, "Cannot stat '%s': %s\n", path, err) + os.Exit(1) + } + + return fi2.IsDir() +} + // Recursively plants a symlink forest at forestDir. The symlink tree will // contain every file in buildFilesDir and srcDir excluding the files in // exclude. Collects every directory encountered during the traversal of srcDir @@ -145,8 +159,18 @@ func plantSymlinkForestRecursive(topdir string, forestDir string, buildFilesDir continue } + sDir := false + bDir := false + if sExists { + sDir = isDir(shared.JoinPath(topdir, srcChild), srcChildEntry) + } + + if bExists { + bDir = isDir(shared.JoinPath(topdir, buildFilesChild), buildFilesChildEntry) + } + if !sExists { - if buildFilesChildEntry.IsDir() && excludeChild != nil { + if bDir && excludeChild != nil { // Not in the source tree, but we have to exclude something from under // this subtree, so descend plantSymlinkForestRecursive(topdir, forestChild, buildFilesChild, srcChild, excludeChild, acc, okay) @@ -155,7 +179,7 @@ func plantSymlinkForestRecursive(topdir string, forestDir string, buildFilesDir symlinkIntoForest(topdir, forestChild, buildFilesChild) } } else if !bExists { - if srcChildEntry.IsDir() && excludeChild != nil { + if sDir && excludeChild != nil { // Not in the build file tree, but we have to exclude something from // under this subtree, so descend plantSymlinkForestRecursive(topdir, forestChild, buildFilesChild, srcChild, excludeChild, acc, okay) @@ -163,10 +187,10 @@ func plantSymlinkForestRecursive(topdir string, forestDir string, buildFilesDir // Not in the build file tree, symlink source tree, carry on symlinkIntoForest(topdir, forestChild, srcChild) } - } else if srcChildEntry.IsDir() && buildFilesChildEntry.IsDir() { + } else if sDir && bDir { // Both are directories. Descend. plantSymlinkForestRecursive(topdir, forestChild, buildFilesChild, srcChild, excludeChild, acc, okay) - } else if !srcChildEntry.IsDir() && !buildFilesChildEntry.IsDir() { + } else if !sDir && !bDir { // Neither is a directory. Prioritize BUILD files generated by bp2build // over any BUILD file imported into external/. fmt.Fprintf(os.Stderr, "Both '%s' and '%s' exist, symlinking the former to '%s'\n", diff --git a/finder/finder.go b/finder/finder.go index b4834b16b..c5196c8de 100644 --- a/finder/finder.go +++ b/finder/finder.go @@ -94,6 +94,10 @@ type CacheParams struct { // RootDirs are the root directories used to initiate the search RootDirs []string + // Whether symlinks are followed. If set, symlinks back to their own parent + // directory don't work. + FollowSymlinks bool + // ExcludeDirs are directory names that if encountered are removed from the search ExcludeDirs []string @@ -1415,9 +1419,14 @@ func (f *Finder) listDirSync(dir *pathMap) { // If stat fails this is probably a broken or dangling symlink, treat it as a file. subfiles = append(subfiles, child.Name()) } else if childStat.IsDir() { - // Skip symlink dirs. - // We don't have to support symlink dirs because - // that would cause duplicates. + // Skip symlink dirs if not requested otherwise. Android has a number + // of symlinks creating infinite source trees which would otherwise get + // us in an infinite loop. + // TODO(b/197349722): Revisit this once symlink loops are banned in the + // source tree. + if f.cacheMetadata.Config.FollowSymlinks { + subdirs = append(subdirs, child.Name()) + } } else { // We do have to support symlink files because the link name might be // different than the target name diff --git a/finder/finder_test.go b/finder/finder_test.go index 788dbdd35..8f73719a6 100644 --- a/finder/finder_test.go +++ b/finder/finder_test.go @@ -90,6 +90,7 @@ func runSimpleTest(t *testing.T, existentPaths []string, expectedMatches []strin CacheParams{ "/cwd", []string{root}, + false, nil, nil, []string{"findme.txt", "skipme.txt"}, @@ -121,6 +122,7 @@ func runTestWithSuffixes(t *testing.T, existentPaths []string, expectedMatches [ CacheParams{ "/cwd", []string{root}, + false, nil, nil, []string{"findme.txt", "skipme.txt"}, diff --git a/tests/bp2build_bazel_test.sh b/tests/bp2build_bazel_test.sh index 4f37c2bff..74e49aabe 100755 --- a/tests/bp2build_bazel_test.sh +++ b/tests/bp2build_bazel_test.sh @@ -115,3 +115,57 @@ EOF } test_bp2build_generates_all_buildfiles + +function test_cc_correctness { + setup + create_mock_bazel + + mkdir -p a + cat > a/Android.bp < a/qq.cc < a/qq.h < a/qq.h <