From ffc4cc41a36ab55e8ac0adc78a22e9ec0712f535 Mon Sep 17 00:00:00 2001 From: Rupert Shuttleworth Date: Wed, 3 Nov 2021 09:07:26 +0000 Subject: [PATCH] Revert "Fix python_binary_host module in mixed build." Revert submission 1861755-fix_python_binary_host_in_mixed_build Reason for revert: Broke Bazel CI, please see https://android-build.googleplex.com/builds/submitted/7879108/mixed_droid-clean/latest/view/logs/build_error.log and https://android-build.googleplex.com/builds/submitted/7879108/mixed_droid-incremental/latest/view/logs/build_error.log Reverted Changes: Ica457ee71:Add platform needed by python_binary_host. Ibc8b99a92:Fix python_binary_host module in mixed build. Change-Id: I3d6c4f6edee2d00bdc78efd5cee64f4c915baeaa --- android/bazel.go | 8 +- bazel/aquery.go | 122 +---------------- bazel/aquery_test.go | 320 ------------------------------------------- python/binary.go | 2 +- python/installer.go | 17 +-- python/test.go | 2 +- 6 files changed, 13 insertions(+), 458 deletions(-) diff --git a/android/bazel.go b/android/bazel.go index 22fbd4d4f..5dda65537 100644 --- a/android/bazel.go +++ b/android/bazel.go @@ -308,8 +308,12 @@ var ( // Per-module denylist to opt modules out of mixed builds. Such modules will // still be generated via bp2build. mixedBuildsDisabledList = []string{ - "libbrotli", // http://b/198585397, ld.lld: error: bionic/libc/arch-arm64/generic/bionic/memmove.S:95:(.text+0x10): relocation R_AARCH64_CONDBR19 out of range: -1404176 is not in [-1048576, 1048575]; references __memcpy - "minijail_constants_json", // http://b/200899432, bazel-built cc_genrule does not work in mixed build when it is a dependency of another soong module. + "libbrotli", // http://b/198585397, ld.lld: error: bionic/libc/arch-arm64/generic/bionic/memmove.S:95:(.text+0x10): relocation R_AARCH64_CONDBR19 out of range: -1404176 is not in [-1048576, 1048575]; references __memcpy + "func_to_syscall_nrs", // http://b/200899432, bazel-built cc_genrule does not work in mixed build when it is a dependency of another soong module. + "libseccomp_policy_app_zygote_sources", // http://b/200899432, bazel-built cc_genrule does not work in mixed build when it is a dependency of another soong module. + "libseccomp_policy_app_sources", // http://b/200899432, bazel-built cc_genrule does not work in mixed build when it is a dependency of another soong module. + "libseccomp_policy_system_sources", // http://b/200899432, bazel-built cc_genrule does not work in mixed build when it is a dependency of another soong module. + "minijail_constants_json", // http://b/200899432, bazel-built cc_genrule does not work in mixed build when it is a dependency of another soong module. } // Used for quicker lookups diff --git a/bazel/aquery.go b/bazel/aquery.go index 3ce86ce72..0dedcf492 100644 --- a/bazel/aquery.go +++ b/bazel/aquery.go @@ -18,7 +18,6 @@ import ( "encoding/json" "fmt" "path/filepath" - "regexp" "strings" "github.com/google/blueprint/proptools" @@ -60,8 +59,6 @@ type action struct { InputDepSetIds []int Mnemonic string OutputIds []int - TemplateContent string - Substitutions []KeyValuePair } // actionGraphContainer contains relevant portions of Bazel's aquery proto, ActionGraphContainer. @@ -103,14 +100,6 @@ type aqueryArtifactHandler struct { artifactIdToPath map[int]string } -// The tokens should be substituted with the value specified here, instead of the -// one returned in 'substitutions' of TemplateExpand action. -var TemplateActionOverriddenTokens = map[string]string{ - // Uses "python3" for %python_binary% instead of the value returned by aquery - // which is "py3wrapper.sh". See removePy3wrapperScript. - "%python_binary%": "python3", -} - func newAqueryHandler(aqueryResult actionGraphContainer) (*aqueryArtifactHandler, error) { pathFragments := map[int]pathFragment{} for _, pathFragment := range aqueryResult.PathFragments { @@ -174,22 +163,7 @@ func (a *aqueryArtifactHandler) getInputPaths(depsetIds []int) ([]string, error) } } } - - // Filter out py3wrapper.sh & MANIFEST file. The middleman action returned by aquery - // for python binary is the input list for a dependent of python binary, since py3wrapper.sh - // and MANIFEST file could not be created in mixed build, they should be removed from - // the input paths here. - py3wrapper := "/py3wrapper.sh" - manifestFile := regexp.MustCompile(".*/.+\\.runfiles/MANIFEST$") - filteredInputPaths := []string{} - for _, path := range inputPaths { - if strings.HasSuffix(path, py3wrapper) || manifestFile.MatchString(path) { - continue - } - filteredInputPaths = append(filteredInputPaths, path) - } - - return filteredInputPaths, nil + return inputPaths, nil } func (a *aqueryArtifactHandler) artifactIdsFromDepsetId(depsetId int) ([]int, error) { @@ -275,21 +249,6 @@ func AqueryBuildStatements(aqueryJsonProto []byte) ([]BuildStatement, error) { // Use hard links, because some soong actions expect real files (for example, `cp -d`). buildStatement.Command = fmt.Sprintf("mkdir -p %[1]s && rm -f %[2]s && ln -f %[3]s %[2]s", outDir, out, in) buildStatement.SymlinkPaths = outputPaths[:] - } else if isTemplateExpandAction(actionEntry) && len(actionEntry.Arguments) < 1 { - if len(outputPaths) != 1 { - return nil, fmt.Errorf("Expect 1 output to template expand action, got: output %q", outputPaths) - } - expandedTemplateContent := expandTemplateContent(actionEntry) - command := fmt.Sprintf(`echo "%[1]s" | sed "s/\\\\n/\\n/g" >> %[2]s && chmod a+x %[2]s`, - escapeCommandlineArgument(expandedTemplateContent), outputPaths[0]) - buildStatement.Command = command - } else if isPythonZipperAction(actionEntry) { - if len(inputPaths) < 1 || len(outputPaths) != 1 { - return nil, fmt.Errorf("Expect 1+ input and 1 output to python zipper action, got: input %q, output %q", inputPaths, outputPaths) - } - buildStatement.InputPaths, buildStatement.Command = removePy3wrapperScript(buildStatement) - buildStatement.Command = addCommandForPyBinaryRunfilesDir(buildStatement, inputPaths[0], outputPaths[0]) - addPythonZipFileAsDependencyOfPythonBinary(&buildStatements, outputPaths[0]) } else if len(actionEntry.Arguments) < 1 { return nil, fmt.Errorf("received action with no command: [%v]", buildStatement) } @@ -299,89 +258,10 @@ func AqueryBuildStatements(aqueryJsonProto []byte) ([]BuildStatement, error) { return buildStatements, nil } -// expandTemplateContent substitutes the tokens in a template. -func expandTemplateContent(actionEntry action) string { - replacerString := []string{} - for _, pair := range actionEntry.Substitutions { - value := pair.Value - if val, ok := TemplateActionOverriddenTokens[pair.Key]; ok { - value = val - } - replacerString = append(replacerString, pair.Key, value) - } - replacer := strings.NewReplacer(replacerString...) - return replacer.Replace(actionEntry.TemplateContent) -} - -func escapeCommandlineArgument(str string) string { - // \->\\, $->\$, `->\`, "->\", \n->\\n - replacer := strings.NewReplacer( - `\`, `\\`, - `$`, `\$`, - "`", "\\`", - `"`, `\"`, - "\n", "\\n", - ) - return replacer.Replace(str) -} - -// removePy3wrapperScript removes py3wrapper.sh from the input paths and command of the action of -// creating python zip file in mixed build. py3wrapper.sh is returned as input by aquery but -// there is no action returned by aquery for creating it. So in mixed build "python3" is used -// as the PYTHON_BINARY in python binary stub script, and py3wrapper.sh is not needed and should be -// removed from input paths and command of creating python zip file. -func removePy3wrapperScript(bs BuildStatement) (newInputPaths []string, newCommand string) { - // Remove from inputs - py3wrapper := "/py3wrapper.sh" - filteredInputPaths := []string{} - for _, path := range bs.InputPaths { - if !strings.HasSuffix(path, py3wrapper) { - filteredInputPaths = append(filteredInputPaths, path) - } - } - newInputPaths = filteredInputPaths - - // Remove from command line - var re = regexp.MustCompile(`\S*` + py3wrapper) - newCommand = re.ReplaceAllString(bs.Command, "") - return -} - -// addCommandForPyBinaryRunfilesDir adds commands creating python binary runfiles directory -// which currently could not be created with aquery output. -func addCommandForPyBinaryRunfilesDir(bs BuildStatement, zipperCommandPath, zipFilePath string) string { - // Unzip the zip file, zipFilePath looks like .zip - runfilesDirName := zipFilePath[0:len(zipFilePath)-4] + ".runfiles" - command := fmt.Sprintf("%s x %s -d %s", zipperCommandPath, zipFilePath, runfilesDirName) - // Create a symblic link in .runfile/, which is the expected structure - // when running the python binary stub script. - command += fmt.Sprintf(" && ln -sf runfiles/__main__ %s", runfilesDirName) - return bs.Command + " && " + command -} - -// addPythonZipFileAsDependencyOfPythonBinary adds the action of generating python zip file as dependency of -// the corresponding action of creating python binary stub script. In mixed build the dependent of python binary depends on -// the action of createing python binary stub script only, which is not sufficient without the python zip file created. -func addPythonZipFileAsDependencyOfPythonBinary(buildStatements *[]BuildStatement, pythonZipFilePath string) { - for i, _ := range *buildStatements { - if len((*buildStatements)[i].OutputPaths) >= 1 && (*buildStatements)[i].OutputPaths[0]+".zip" == pythonZipFilePath { - (*buildStatements)[i].InputPaths = append((*buildStatements)[i].InputPaths, pythonZipFilePath) - } - } -} - func isSymlinkAction(a action) bool { return a.Mnemonic == "Symlink" || a.Mnemonic == "SolibSymlink" } -func isTemplateExpandAction(a action) bool { - return a.Mnemonic == "TemplateExpand" -} - -func isPythonZipperAction(a action) bool { - return a.Mnemonic == "PythonZipper" -} - func shouldSkipAction(a action) bool { // TODO(b/180945121): Handle complex symlink actions. if a.Mnemonic == "SymlinkTree" || a.Mnemonic == "SourceSymlinkManifest" { diff --git a/bazel/aquery_test.go b/bazel/aquery_test.go index f94d73ffe..88066c8fe 100644 --- a/bazel/aquery_test.go +++ b/bazel/aquery_test.go @@ -1015,326 +1015,6 @@ func TestSymlinkMultipleOutputs(t *testing.T) { assertError(t, err, `Expect 1 input and 1 output to symlink action, got: input ["file"], output ["symlink" "other_symlink"]`) } -func TestTemplateExpandActionSubstitutions(t *testing.T) { - const inputString = ` -{ - "artifacts": [{ - "id": 1, - "pathFragmentId": 1 - }], - "actions": [{ - "targetId": 1, - "actionKey": "x", - "mnemonic": "TemplateExpand", - "configurationId": 1, - "outputIds": [1], - "primaryOutputId": 1, - "executionPlatform": "//build/bazel/platforms:linux_x86_64", - "templateContent": "Test template substitutions: %token1%, %python_binary%", - "substitutions": [{ - "key": "%token1%", - "value": "abcd" - },{ - "key": "%python_binary%", - "value": "python3" - }] - }], - "pathFragments": [{ - "id": 1, - "label": "template_file" - }] -}` - - actual, err := AqueryBuildStatements([]byte(inputString)) - - if err != nil { - t.Errorf("Unexpected error %q", err) - } - - expectedBuildStatements := []BuildStatement{ - BuildStatement{ - Command: "echo \"Test template substitutions: abcd, python3\" | sed \"s/\\\\\\\\n/\\\\n/g\" >> template_file && " + - "chmod a+x template_file", - OutputPaths: []string{"template_file"}, - Mnemonic: "TemplateExpand", - }, - } - assertBuildStatements(t, expectedBuildStatements, actual) -} - -func TestTemplateExpandActionNoOutput(t *testing.T) { - const inputString = ` -{ - "artifacts": [{ - "id": 1, - "pathFragmentId": 1 - }], - "actions": [{ - "targetId": 1, - "actionKey": "x", - "mnemonic": "TemplateExpand", - "configurationId": 1, - "primaryOutputId": 1, - "executionPlatform": "//build/bazel/platforms:linux_x86_64", - "templateContent": "Test template substitutions: %token1%, %python_binary%", - "substitutions": [{ - "key": "%token1%", - "value": "abcd" - },{ - "key": "%python_binary%", - "value": "python3" - }] - }], - "pathFragments": [{ - "id": 1, - "label": "template_file" - }] -}` - - _, err := AqueryBuildStatements([]byte(inputString)) - assertError(t, err, `Expect 1 output to template expand action, got: output []`) -} - -func TestPythonZipperActionSuccess(t *testing.T) { - const inputString = ` -{ - "artifacts": [{ - "id": 1, - "pathFragmentId": 1 - },{ - "id": 2, - "pathFragmentId": 2 - },{ - "id": 3, - "pathFragmentId": 3 - },{ - "id": 4, - "pathFragmentId": 4 - },{ - "id": 5, - "pathFragmentId": 10 - }], - "actions": [{ - "targetId": 1, - "actionKey": "x", - "mnemonic": "PythonZipper", - "configurationId": 1, - "arguments": ["../bazel_tools/tools/zip/zipper/zipper", "cC", "python_binary.zip", "__main__.py\u003dbazel-out/k8-fastbuild/bin/python_binary.temp", "__init__.py\u003d", "runfiles/__main__/__init__.py\u003d", "runfiles/__main__/python_binary.py\u003dpython_binary.py", "runfiles/bazel_tools/tools/python/py3wrapper.sh\u003dbazel-out/bazel_tools/k8-fastbuild/bin/tools/python/py3wrapper.sh"], - "outputIds": [2], - "inputDepSetIds": [1], - "primaryOutputId": 2 - }], - "depSetOfFiles": [{ - "id": 1, - "directArtifactIds": [4, 3, 5] - }], - "pathFragments": [{ - "id": 1, - "label": "python_binary" - },{ - "id": 2, - "label": "python_binary.zip" - },{ - "id": 3, - "label": "python_binary.py" - },{ - "id": 9, - "label": ".." - }, { - "id": 8, - "label": "bazel_tools", - "parentId": 9 - }, { - "id": 7, - "label": "tools", - "parentId": 8 - }, { - "id": 6, - "label": "zip", - "parentId": 7 - }, { - "id": 5, - "label": "zipper", - "parentId": 6 - }, { - "id": 4, - "label": "zipper", - "parentId": 5 - },{ - "id": 16, - "label": "bazel-out" - },{ - "id": 15, - "label": "bazel_tools", - "parentId": 16 - }, { - "id": 14, - "label": "k8-fastbuild", - "parentId": 15 - }, { - "id": 13, - "label": "bin", - "parentId": 14 - }, { - "id": 12, - "label": "tools", - "parentId": 13 - }, { - "id": 11, - "label": "python", - "parentId": 12 - }, { - "id": 10, - "label": "py3wrapper.sh", - "parentId": 11 - }] -}` - actual, err := AqueryBuildStatements([]byte(inputString)) - - if err != nil { - t.Errorf("Unexpected error %q", err) - } - - expectedBuildStatements := []BuildStatement{ - BuildStatement{ - Command: "../bazel_tools/tools/zip/zipper/zipper cC python_binary.zip __main__.py=bazel-out/k8-fastbuild/bin/python_binary.temp " + - "__init__.py= runfiles/__main__/__init__.py= runfiles/__main__/python_binary.py=python_binary.py && " + - "../bazel_tools/tools/zip/zipper/zipper x python_binary.zip -d python_binary.runfiles && ln -sf runfiles/__main__ python_binary.runfiles", - InputPaths: []string{"../bazel_tools/tools/zip/zipper/zipper", "python_binary.py"}, - OutputPaths: []string{"python_binary.zip"}, - Mnemonic: "PythonZipper", - }, - } - assertBuildStatements(t, expectedBuildStatements, actual) -} - -func TestPythonZipperActionNoInput(t *testing.T) { - const inputString = ` -{ - "artifacts": [{ - "id": 1, - "pathFragmentId": 1 - },{ - "id": 2, - "pathFragmentId": 2 - }], - "actions": [{ - "targetId": 1, - "actionKey": "x", - "mnemonic": "PythonZipper", - "configurationId": 1, - "arguments": ["../bazel_tools/tools/zip/zipper/zipper", "cC", "python_binary.zip", "__main__.py\u003dbazel-out/k8-fastbuild/bin/python_binary.temp", "__init__.py\u003d", "runfiles/__main__/__init__.py\u003d", "runfiles/__main__/python_binary.py\u003dpython_binary.py", "runfiles/bazel_tools/tools/python/py3wrapper.sh\u003dbazel-out/bazel_tools/k8-fastbuild/bin/tools/python/py3wrapper.sh"], - "outputIds": [2], - "primaryOutputId": 2 - }], - "pathFragments": [{ - "id": 1, - "label": "python_binary" - },{ - "id": 2, - "label": "python_binary.zip" - }] -}` - _, err := AqueryBuildStatements([]byte(inputString)) - assertError(t, err, `Expect 1+ input and 1 output to python zipper action, got: input [], output ["python_binary.zip"]`) -} - -func TestPythonZipperActionNoOutput(t *testing.T) { - const inputString = ` -{ - "artifacts": [{ - "id": 1, - "pathFragmentId": 1 - },{ - "id": 2, - "pathFragmentId": 2 - },{ - "id": 3, - "pathFragmentId": 3 - },{ - "id": 4, - "pathFragmentId": 4 - },{ - "id": 5, - "pathFragmentId": 10 - }], - "actions": [{ - "targetId": 1, - "actionKey": "x", - "mnemonic": "PythonZipper", - "configurationId": 1, - "arguments": ["../bazel_tools/tools/zip/zipper/zipper", "cC", "python_binary.zip", "__main__.py\u003dbazel-out/k8-fastbuild/bin/python_binary.temp", "__init__.py\u003d", "runfiles/__main__/__init__.py\u003d", "runfiles/__main__/python_binary.py\u003dpython_binary.py", "runfiles/bazel_tools/tools/python/py3wrapper.sh\u003dbazel-out/bazel_tools/k8-fastbuild/bin/tools/python/py3wrapper.sh"], - "inputDepSetIds": [1] - }], - "depSetOfFiles": [{ - "id": 1, - "directArtifactIds": [4, 3, 5] - }], - "pathFragments": [{ - "id": 1, - "label": "python_binary" - },{ - "id": 2, - "label": "python_binary.zip" - },{ - "id": 3, - "label": "python_binary.py" - },{ - "id": 9, - "label": ".." - }, { - "id": 8, - "label": "bazel_tools", - "parentId": 9 - }, { - "id": 7, - "label": "tools", - "parentId": 8 - }, { - "id": 6, - "label": "zip", - "parentId": 7 - }, { - "id": 5, - "label": "zipper", - "parentId": 6 - }, { - "id": 4, - "label": "zipper", - "parentId": 5 - },{ - "id": 16, - "label": "bazel-out" - },{ - "id": 15, - "label": "bazel_tools", - "parentId": 16 - }, { - "id": 14, - "label": "k8-fastbuild", - "parentId": 15 - }, { - "id": 13, - "label": "bin", - "parentId": 14 - }, { - "id": 12, - "label": "tools", - "parentId": 13 - }, { - "id": 11, - "label": "python", - "parentId": 12 - }, { - "id": 10, - "label": "py3wrapper.sh", - "parentId": 11 - }] -}` - _, err := AqueryBuildStatements([]byte(inputString)) - assertError(t, err, `Expect 1+ input and 1 output to python zipper action, got: input ["../bazel_tools/tools/zip/zipper/zipper" "python_binary.py"], output []`) -} - func assertError(t *testing.T, err error, expected string) { t.Helper() if err == nil { diff --git a/python/binary.go b/python/binary.go index 304c9a98b..bf6167c3c 100644 --- a/python/binary.go +++ b/python/binary.go @@ -143,7 +143,7 @@ var ( func NewBinary(hod android.HostOrDeviceSupported) (*Module, *binaryDecorator) { module := newModule(hod, android.MultilibFirst) - decorator := &binaryDecorator{pythonInstaller: NewPythonInstaller("bin", "", module)} + decorator := &binaryDecorator{pythonInstaller: NewPythonInstaller("bin", "")} module.bootstrapper = decorator module.installer = decorator diff --git a/python/installer.go b/python/installer.go index 515cc47f2..396f03667 100644 --- a/python/installer.go +++ b/python/installer.go @@ -36,14 +36,12 @@ type pythonInstaller struct { path android.InstallPath androidMkSharedLibs []string - module *Module } -func NewPythonInstaller(dir, dir64 string, module *Module) *pythonInstaller { +func NewPythonInstaller(dir, dir64 string) *pythonInstaller { return &pythonInstaller{ - dir: dir, - dir64: dir64, - module: module, + dir: dir, + dir64: dir64, } } @@ -61,14 +59,7 @@ func (installer *pythonInstaller) installDir(ctx android.ModuleContext) android. } func (installer *pythonInstaller) install(ctx android.ModuleContext, file android.Path) { - if ctx.ModuleType() == "python_binary_host" && installer.module.MixedBuildsEnabled(ctx) { - label := installer.module.BazelModuleBase.GetBazelLabel(ctx, installer.module) - binary, _ := ctx.Config().BazelContext.GetPythonBinary(label, android.GetConfigKey(ctx)) - bazelBinaryOutPath := android.PathForBazelOut(ctx, binary) - installer.path = ctx.InstallFile(installer.installDir(ctx), bazelBinaryOutPath.Base(), bazelBinaryOutPath) - } else { - installer.path = ctx.InstallFile(installer.installDir(ctx), file.Base(), file) - } + installer.path = ctx.InstallFile(installer.installDir(ctx), file.Base(), file) } func (installer *pythonInstaller) setAndroidMkSharedLibs(sharedLibs []string) { diff --git a/python/test.go b/python/test.go index 3cd900f4c..7413782cb 100644 --- a/python/test.go +++ b/python/test.go @@ -101,7 +101,7 @@ func (test *testDecorator) install(ctx android.ModuleContext, file android.Path) func NewTest(hod android.HostOrDeviceSupported) *Module { module, binary := NewBinary(hod) - binary.pythonInstaller = NewPythonInstaller("nativetest", "nativetest64", module) + binary.pythonInstaller = NewPythonInstaller("nativetest", "nativetest64") test := &testDecorator{binaryDecorator: binary} if hod == android.HostSupportedNoCross && test.testProperties.Test_options.Unit_test == nil {