From 325c70114376c63dad6077a5eeafc93bac89c340 Mon Sep 17 00:00:00 2001 From: Yun Peng Date: Thu, 30 Aug 2018 16:39:45 +0200 Subject: [PATCH] Windows: Make Python binary work with runfiles symlink tree Change-Id: Ibdd03de616e727286eb2a53ca5b08d46e76b2b1c --- .../rules/python/BazelPythonSemantics.java | 16 ++++++++++--- .../rules/python/python_stub_template.txt | 17 +++++++++---- .../build/lib/rules/python/PyCommon.java | 9 +++++++ .../shell/bazel/bazel_windows_example_test.sh | 24 +++++++++++++++++++ src/tools/launcher/launcher.cc | 3 +-- src/tools/launcher/python_launcher.cc | 11 +++++++-- 6 files changed, 69 insertions(+), 11 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java index 9a71ae9f553b68..4449b1dd748fce 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java @@ -144,10 +144,19 @@ public Artifact createExecutable( String pythonBinary = getPythonBinary(ruleContext, config); if (!ruleContext.getFragment(PythonConfiguration.class).buildPythonZip()) { + Artifact stubOutput = executable; + if (OS.getCurrent() == OS.WINDOWS + && ruleContext.getConfiguration().enableWindowsExeLauncher()) { + // On Windows, use a Windows native binary to launch the python launcher script (stub file). + stubOutput = common.getPythonLauncherArtifact(executable); + executable = createWindowsExeLauncher( + ruleContext, pythonBinary, executable, /*useZipFile*/ false); + } + ruleContext.registerAction( new TemplateExpansionAction( ruleContext.getActionOwner(), - executable, + stubOutput, STUB_TEMPLATE, ImmutableList.of( Substitution.of("%main%", main), @@ -194,7 +203,7 @@ public Artifact createExecutable( .build(ruleContext)); } else { if (ruleContext.getConfiguration().enableWindowsExeLauncher()) { - return createWindowsExeLauncher(ruleContext, pythonBinary, executable); + return createWindowsExeLauncher(ruleContext, pythonBinary, executable, true); } ruleContext.registerAction( @@ -212,13 +221,14 @@ public Artifact createExecutable( } private static Artifact createWindowsExeLauncher( - RuleContext ruleContext, String pythonBinary, Artifact pythonLauncher) + RuleContext ruleContext, String pythonBinary, Artifact pythonLauncher, boolean useZipFile) throws InterruptedException { LaunchInfo launchInfo = LaunchInfo.builder() .addKeyValuePair("binary_type", "Python") .addKeyValuePair("workspace_name", ruleContext.getWorkspaceName()) .addKeyValuePair("python_bin_path", pythonBinary) + .addKeyValuePair("use_zip_file", useZipFile ? "1" : "0") .build(); LauncherFileWriteAction.createAndRegister(ruleContext, pythonLauncher, launchInfo); return pythonLauncher; diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt index d2cdccc6fd1be9..8acdf292c218f4 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/python_stub_template.txt @@ -55,10 +55,10 @@ def FindPythonBinary(module_space): # Case 1: Path is a label. Not supported yet. raise AssertionError( 'Bazel does not support execution of Python interpreters via labels yet') - elif PYTHON_BINARY.startswith('/'): + elif os.path.isabs(PYTHON_BINARY): # Case 2: Absolute path. return PYTHON_BINARY - elif '/' in PYTHON_BINARY: + elif os.sep in PYTHON_BINARY: # Case 3: Path is relative to the repo root. return os.path.join(module_space, PYTHON_BINARY) else: @@ -71,8 +71,12 @@ def CreatePythonPathEntries(python_imports, module_space): # Find the runfiles tree def FindModuleSpace(): + runfiles_dir = os.environ.get('RUNFILES_DIR', None) + if runfiles_dir: + return runfiles_dir + stub_filename = os.path.abspath(sys.argv[0]) - module_space = stub_filename + '.runfiles' + module_space = stub_filename + ('.exe' if IsWindows() else '') + '.runfiles' if os.path.isdir(module_space): return module_space @@ -190,7 +194,12 @@ def Main(): shutil.rmtree(os.path.dirname(module_space), True) exit(retCode) else: - os.execv(args[0], args) + if IsWindows(): + # On Windows, os.execv doesn't handle arguments with spaces correctly, and it + # actually starts a subprocess just like subprocess.call. + exit(subprocess.call(args)) + else: + os.execv(args[0], args) except EnvironmentError: # This works from Python 2.4 all the way to 3.x. e = sys.exc_info()[1] diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java index 4483d7e063a232..be2e5fca0e0fd3 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java @@ -138,6 +138,10 @@ public void initBinary(List srcs) { if (ruleContext.getFragment(PythonConfiguration.class).buildPythonZip()) { filesToBuildBuilder.add(getPythonZipArtifact(executable)); + } else if (OS.getCurrent() == OS.WINDOWS) { + // TODO(bazel-team): Here we should check target platform instead of using OS.getCurrent(). + // On Windows, add the python stub launcher in the set of files to build. + filesToBuildBuilder.add(getPythonLauncherArtifact(executable)); } filesToBuild = filesToBuildBuilder.build(); @@ -154,6 +158,11 @@ public Artifact getPythonZipArtifact(Artifact executable) { return ruleContext.getRelatedArtifact(executable.getRootRelativePath(), ".zip"); } + /** @return An artifact next to the executable file with no suffix, only used on Windows */ + public Artifact getPythonLauncherArtifact(Artifact executable) { + return ruleContext.getRelatedArtifact(executable.getRootRelativePath(), ""); + } + public void addCommonTransitiveInfoProviders(RuleConfiguredTargetBuilder builder, PythonSemantics semantics, NestedSet filesToBuild) { diff --git a/src/test/shell/bazel/bazel_windows_example_test.sh b/src/test/shell/bazel/bazel_windows_example_test.sh index 3779510abcd2a6..5b7eeaa44a3701 100755 --- a/src/test/shell/bazel/bazel_windows_example_test.sh +++ b/src/test/shell/bazel/bazel_windows_example_test.sh @@ -196,6 +196,30 @@ function test_native_python() { assert_test_fails //examples/py_native:fail } +function test_native_python_with_runfiles() { + BUILD_FLAGS="--experimental_enable_runfiles --build_python_zip=0" + bazel build -s --verbose_failures $BUILD_FLAGS //examples/py_native:bin \ + || fail "Failed to build //examples/py_native:bin with runfiles support" + ( + # Clear runfiles related env + unset RUNFILES_MANIFEST_FILE + unset RUNFILES_MANIFEST_ONLY + unset RUNFILES_DIR + # Run the python package directly + ./bazel-bin/examples/py_native/bin >& $TEST_log \ + || fail "//examples/py_native:bin execution failed" + expect_log "Fib(5) == 8" + # Use python to run the python package + python ./bazel-bin/examples/py_native/bin >& $TEST_log \ + || fail "//examples/py_native:bin execution failed" + expect_log "Fib(5) == 8" ) + bazel test --test_output=errors $BUILD_FLAGS //examples/py_native:test >> $TEST_log 2>&1 \ + || fail "Test //examples/py_native:test failed while expecting success" + bazel test --test_output=errors $BUILD_FLAGS //examples/py_native:fail >> $TEST_log 2>&1 \ + && fail "Test //examples/py_native:fail succeed while expecting failure" \ + || true +} + function test_native_python_with_python3() { PYTHON3_PATH=${PYTHON3_PATH:-/c/Program Files/Anaconda3} if [ ! -x "${PYTHON3_PATH}/python.exe" ]; then diff --git a/src/tools/launcher/launcher.cc b/src/tools/launcher/launcher.cc index e1a96ab40a891b..70dd2429636765 100644 --- a/src/tools/launcher/launcher.cc +++ b/src/tools/launcher/launcher.cc @@ -229,9 +229,8 @@ ExitCode BinaryLauncherBase::LaunchProcess(const wstring& executable, if (!manifest_file.empty()) { SetEnv(L"RUNFILES_MANIFEST_ONLY", L"1"); SetEnv(L"RUNFILES_MANIFEST_FILE", manifest_file); - } else { - SetEnv(L"RUNFILES_DIR", runfiles_dir); } + SetEnv(L"RUNFILES_DIR", runfiles_dir); CmdLine cmdline; CreateCommandLine(&cmdline, executable, arguments); PROCESS_INFORMATION processInfo = {0}; diff --git a/src/tools/launcher/python_launcher.cc b/src/tools/launcher/python_launcher.cc index a0f8775923d97c..8afc9d621d9e22 100644 --- a/src/tools/launcher/python_launcher.cc +++ b/src/tools/launcher/python_launcher.cc @@ -25,6 +25,7 @@ using std::vector; using std::wstring; static constexpr const char* PYTHON_BIN_PATH = "python_bin_path"; +static constexpr const char* USE_ZIP_FILE = "use_zip_file"; ExitCode PythonBinaryLauncher::Launch() { wstring python_binary = this->GetLaunchInfoByKey(PYTHON_BIN_PATH); @@ -35,10 +36,16 @@ ExitCode PythonBinaryLauncher::Launch() { } vector args = this->GetCommandlineArguments(); - wstring python_zip_file = GetBinaryPathWithoutExtension(args[0]) + L".zip"; + wstring use_zip_file = this->GetLaunchInfoByKey(USE_ZIP_FILE); + wstring python_file; + if (use_zip_file == L"1") { + python_file = GetBinaryPathWithoutExtension(args[0]) + L".zip"; + } else { + python_file = GetBinaryPathWithoutExtension(args[0]); + } // Replace the first argument with python zip file path - args[0] = python_zip_file; + args[0] = python_file; // Escape arguments that has spaces for (int i = 1; i < args.size(); i++) {