diff options
author | 2022-05-02 10:13:19 +0200 | |
---|---|---|
committer | 2022-05-04 09:12:01 +0200 | |
commit | e3487c88482c96b31c8bd5ff4f14d3bc0e70b784 (patch) | |
tree | 29dae5eb3240ff3ca8e40629f2edfc6f664dc918 | |
parent | a704eb14690da4533c970e807a4224268cb499a0 (diff) |
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
-rw-r--r-- | android/variable.go | 13 | ||||
-rw-r--r-- | bp2build/symlink_forest.go | 32 | ||||
-rw-r--r-- | finder/finder.go | 15 | ||||
-rw-r--r-- | finder/finder_test.go | 2 | ||||
-rwxr-xr-x | tests/bp2build_bazel_test.sh | 54 | ||||
-rw-r--r-- | tests/lib.sh | 4 | ||||
-rw-r--r-- | 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 <<EOF +cc_object { + name: "qq", + srcs: ["qq.cc"], + bazel_module: { + bp2build_available: true, + }, + stl: "none", + system_shared_libs: [], +} +EOF + + cat > a/qq.cc <<EOF +#include "qq.h" +int qq() { + return QQ; +} +EOF + + cat > a/qq.h <<EOF +#define QQ 1 +EOF + + run_soong bp2build + + run_bazel build --package_path=out/soong/workspace //a:qq + local output_mtime1=$(stat -c "%y" bazel-bin/a/_objs/qq/qq.o) + + run_bazel build --package_path=out/soong/workspace //a:qq + local output_mtime2=$(stat -c "%y" bazel-bin/a/_objs/qq/qq.o) + + if [[ "$output_mtime1" != "$output_mtime2" ]]; then + fail "output changed on null build" + fi + + cat > a/qq.h <<EOF +#define QQ 2 +EOF + + run_bazel build --package_path=out/soong/workspace //a:qq + local output_mtime3=$(stat -c "%y" bazel-bin/a/_objs/qq/qq.o) + + if [[ "$output_mtime1" == "$output_mtime3" ]]; then + fail "output not changed when included header changed" + fi +} + +test_cc_correctness diff --git a/tests/lib.sh b/tests/lib.sh index 1bb2df915..7fd970a40 100644 --- a/tests/lib.sh +++ b/tests/lib.sh @@ -85,6 +85,7 @@ function create_mock_soong { copy_directory build/soong copy_directory build/make/tools/rbcrun + symlink_directory prebuilts/sdk symlink_directory prebuilts/go symlink_directory prebuilts/build-tools symlink_directory prebuilts/clang/host @@ -115,8 +116,10 @@ function create_mock_bazel() { copy_directory build/bazel symlink_directory prebuilts/bazel + symlink_directory prebuilts/clang symlink_directory prebuilts/jdk symlink_directory external/bazel-skylib + symlink_directory external/bazelbuild-rules_android symlink_file WORKSPACE symlink_file BUILD @@ -136,4 +139,5 @@ info "Mock top: $MOCK_TOP" export ALLOW_MISSING_DEPENDENCIES=true +export ALLOW_BP_UNDER_SYMLINKS=true warmup_mock_top diff --git a/ui/build/finder.go b/ui/build/finder.go index 262de3de7..4d6ad426f 100644 --- a/ui/build/finder.go +++ b/ui/build/finder.go @@ -64,6 +64,7 @@ func NewSourceFinder(ctx Context, config Config) (f *finder.Finder) { cacheParams := finder.CacheParams{ WorkingDirectory: dir, RootDirs: []string{"."}, + FollowSymlinks: config.environ.IsEnvTrue("ALLOW_BP_UNDER_SYMLINKS"), ExcludeDirs: []string{".git", ".repo"}, PruneFiles: pruneFiles, IncludeFiles: []string{ |