diff options
-rw-r--r-- | src/main/cpp/startup_options.cc | 37 | ||||
-rw-r--r-- | src/main/cpp/startup_options.h | 4 | ||||
-rw-r--r-- | src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java | 103 | ||||
-rw-r--r-- | src/test/py/bazel/BUILD | 23 | ||||
-rw-r--r-- | src/test/py/bazel/bazel_windows_test.py | 50 | ||||
-rw-r--r-- | src/test/py/bazel/empty_test.py | 27 | ||||
-rw-r--r-- | src/test/py/bazel/test_base.py | 18 |
7 files changed, 180 insertions, 82 deletions
diff --git a/src/main/cpp/startup_options.cc b/src/main/cpp/startup_options.cc index d41d74790a..7b03f69f40 100644 --- a/src/main/cpp/startup_options.cc +++ b/src/main/cpp/startup_options.cc @@ -23,7 +23,6 @@ #include "src/main/cpp/util/errors.h" #include "src/main/cpp/util/exit_code.h" #include "src/main/cpp/util/file.h" -#include "src/main/cpp/util/file_platform.h" #include "src/main/cpp/util/numbers.h" #include "src/main/cpp/util/strings.h" #include "src/main/cpp/workspace_layout.h" @@ -64,6 +63,14 @@ StartupOptions::StartupOptions(const string &product_name, output_root = workspace_layout->GetOutputRoot(); } +#if defined(COMPILER_MSVC) || defined(__CYGWIN__) + string windows_unix_root = WindowsUnixRoot(blaze::GetEnv("BAZEL_SH")); + if (!windows_unix_root.empty()) { + host_jvm_args.push_back(string("-Dbazel.windows_unix_root=") + + windows_unix_root); + } +#endif // defined(COMPILER_MSVC) || defined(__CYGWIN__) + const string product_name_lower = GetLowercaseProductName(); output_user_root = blaze_util::JoinPath( output_root, "_" + product_name_lower + "_" + GetUserName()); @@ -432,4 +439,32 @@ blaze_exit_code::ExitCode StartupOptions::AddJVMArguments( return blaze_exit_code::SUCCESS; } +#if defined(COMPILER_MSVC) || defined(__CYGWIN__) +// Extract the Windows path of "/" from $BAZEL_SH. +// $BAZEL_SH usually has the form `<prefix>/usr/bin/bash.exe` or +// `<prefix>/bin/bash.exe`, and this method returns that `<prefix>` part. +// If $BAZEL_SH doesn't end with "usr/bin/bash.exe" or "bin/bash.exe" then this +// method returns an empty string. +string StartupOptions::WindowsUnixRoot(const string &bazel_sh) { + if (bazel_sh.empty()) { + return string(); + } + std::pair<string, string> split = blaze_util::SplitPath(bazel_sh); + if (blaze_util::AsLower(split.second) != "bash.exe") { + return string(); + } + split = blaze_util::SplitPath(split.first); + if (blaze_util::AsLower(split.second) != "bin") { + return string(); + } + + std::pair<string, string> split2 = blaze_util::SplitPath(split.first); + if (blaze_util::AsLower(split2.second) == "usr") { + return split2.first; + } else { + return split.first; + } +} +#endif // defined(COMPILER_MSVC) || defined(__CYGWIN__) + } // namespace blaze diff --git a/src/main/cpp/startup_options.h b/src/main/cpp/startup_options.h index 2f317c9b3e..607810d71d 100644 --- a/src/main/cpp/startup_options.h +++ b/src/main/cpp/startup_options.h @@ -230,6 +230,10 @@ class StartupOptions { private: std::string host_javabase; std::string default_host_javabase; + +#if defined(COMPILER_MSVC) || defined(__CYGWIN__) + static std::string WindowsUnixRoot(const std::string &bazel_sh); +#endif }; } // namespace blaze diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java index c3a9095082..893a563c3e 100644 --- a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java @@ -27,13 +27,11 @@ import com.google.devtools.build.lib.vfs.PathFragment; import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; -import java.io.InputStream; -import java.io.InputStreamReader; import java.nio.file.Files; import java.nio.file.LinkOption; import java.nio.file.attribute.DosFileAttributes; import java.util.Arrays; -import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import java.util.regex.Matcher; import java.util.regex.Pattern; import javax.annotation.Nullable; @@ -128,32 +126,25 @@ public class WindowsFileSystem extends JavaIoFileSystem { // this heuristic for the valid drives. It's possible that the user has a directory "/a" // but no "A:\" drive, so in that case we should prepend the MSYS root. - // TODO(laszlocsomor): get rid of this heuristic and translate paths using /etc/mtab. - // Figure out how to handle non-top-level mount points (e.g. "/usr/bin" is mounted to - // "/bin"), which is problematic because Paths are created segment by segment, so - // individual Path objects don't know they are parts of a mount point path. - // Another challenge is figuring out how and when to read /etc/mtab. A simple approach is - // to do it in determineUnixRoot, but then we won't pick up mount changes during the - // lifetime of the Bazel server process. A correct approach would be to establish a - // Skyframe FileValue-dependency on it, but it's unclear how this class could request or - // retrieve Skyframe-computed data. - // - // Or wait until https://github.com/bazelbuild/bazel/issues/2107 is fixed and forget about - // Unix paths altogether. :) - if (WindowsPath.isWindowsVolumeName(child)) { child = WindowsPath.getDriveLetter((WindowsPath) parent, child) + ":"; } else { - Preconditions.checkNotNull( - UNIX_ROOT, - "Could not determine Unix path root or it is not an absolute Windows path. Set the " - + "\"%s\" JVM argument, or export the \"%s\" environment variable for the MSYS " - + "bash and have /usr/bin/cygpath installed. Parent is \"%s\", name is \"%s\".", - WINDOWS_UNIX_ROOT_JVM_ARG, - BAZEL_SH_ENV_VAR, - parent, - child); - parent = parent.getRelative(UNIX_ROOT); + if (UNIX_ROOT.get() == null) { + String jvmFlag = "bazel.windows_unix_root"; + PathFragment value = determineUnixRoot(jvmFlag); + if (value == null) { + throw new IllegalStateException( + String.format( + "\"%1$s\" JVM flag is not set. Use the --host_jvm_args flag or export the " + + "BAZEL_SH environment variable. For example " + + "\"--host_jvm_args=-D%1$s=c:/tools/msys64\" or " + + "\"set BAZEL_SH=c:/tools/msys64/usr/bin/bash.exe\". " + + "parent=(%2$s) name=(%3$s)", + jvmFlag, parent, child)); + } + UNIX_ROOT.set(value); + } + parent = parent.getRelative(UNIX_ROOT.get()); } } @@ -305,7 +296,7 @@ public class WindowsFileSystem extends JavaIoFileSystem { @VisibleForTesting @Override - protected void applyToChildren(Predicate<Path> function) { + protected synchronized void applyToChildren(Predicate<Path> function) { super.applyToChildren(function); } } @@ -315,13 +306,7 @@ public class WindowsFileSystem extends JavaIoFileSystem { return WindowsPathFactory.createForTesting(mockResolver); } - private static final String WINDOWS_UNIX_ROOT_JVM_ARG = "bazel.windows_unix_root"; - private static final String BAZEL_SH_ENV_VAR = "BAZEL_SH"; - - // Absolute Windows path specifying the root of absolute Unix paths. - // This is typically the MSYS installation root, e.g. C:\\tools\\msys64 - private static final PathFragment UNIX_ROOT = - determineUnixRoot(WINDOWS_UNIX_ROOT_JVM_ARG, BAZEL_SH_ENV_VAR); + private static final AtomicReference<PathFragment> UNIX_ROOT = new AtomicReference<>(null); public static final LinkOption[] NO_OPTIONS = new LinkOption[0]; public static final LinkOption[] NO_FOLLOW = new LinkOption[] {LinkOption.NOFOLLOW_LINKS}; @@ -491,55 +476,21 @@ public class WindowsFileSystem extends JavaIoFileSystem { file.toPath(), DosFileAttributes.class, symlinkOpts(followSymlinks)); } - private static PathFragment determineUnixRoot(String jvmArgName, String bazelShEnvVar) { - // Get the path from a JVM argument, if specified. + private static PathFragment determineUnixRoot(String jvmArgName) { + // Get the path from a JVM flag, if specified. String path = System.getProperty(jvmArgName); - - if (path == null || path.isEmpty()) { - path = ""; - - // Fall back to executing cygpath. - String bash = System.getenv(bazelShEnvVar); - Process process = null; - try { - process = Runtime.getRuntime().exec("cmd.exe /C " + bash + " -c \"/usr/bin/cygpath -m /\""); - - // Wait 3 seconds max, that should be enough to run this command. - process.waitFor(3, TimeUnit.SECONDS); - - if (process.exitValue() == 0) { - path = readAll(process.getInputStream()); - } else { - System.err.print( - String.format( - "ERROR: %s (exit code: %d)%n", - readAll(process.getErrorStream()), process.exitValue())); - } - } catch (InterruptedException | IOException e) { - // Silently ignore failure. Either MSYS is installed at a different location, or not - // installed at all, or some error occurred. We can't do anything anymore but throw an - // exception if someone tries to create a Path from an absolute Unix path. - return null; - } + if (path == null) { + return null; } path = path.trim(); - PathFragment result = PathFragment.create(path); - if (path.isEmpty() || result.getDriveLetter() == '\0' || !result.isAbsolute()) { + if (path.isEmpty()) { return null; - } else { - return result; } - } - private static String readAll(InputStream s) throws IOException { - String result = ""; - int len; - char[] buf = new char[4096]; - try (InputStreamReader r = new InputStreamReader(s)) { - while ((len = r.read(buf)) > 0) { - result += new String(buf, 0, len); - } + PathFragment result = PathFragment.create(path); + if (result.getDriveLetter() == '\0' || !result.isAbsolute()) { + return null; } return result; } diff --git a/src/test/py/bazel/BUILD b/src/test/py/bazel/BUILD index 428e97d629..32a5adda1e 100644 --- a/src/test/py/bazel/BUILD +++ b/src/test/py/bazel/BUILD @@ -32,3 +32,26 @@ py_test( srcs = ["bazel_clean_test.py"], deps = [":test_base"], ) + +py_test( + name = "bazel_windows_test", + size = "medium", + srcs = select({ + "//src:windows": ["bazel_windows_test.py"], + "//src:windows_msvc": ["bazel_windows_test.py"], + "//src:windows_msys": ["bazel_windows_test.py"], + "//conditions:default": ["empty_test.py"], + }), + main = select({ + "//src:windows": "bazel_windows_test.py", + "//src:windows_msvc": "bazel_windows_test.py", + "//src:windows_msys": "bazel_windows_test.py", + "//conditions:default": "empty_test.py", + }), + deps = select({ + "//src:windows": [":test_base"], + "//src:windows_msvc": [":test_base"], + "//src:windows_msys": [":test_base"], + "//conditions:default": [], + }), +) diff --git a/src/test/py/bazel/bazel_windows_test.py b/src/test/py/bazel/bazel_windows_test.py new file mode 100644 index 0000000000..27bc37366c --- /dev/null +++ b/src/test/py/bazel/bazel_windows_test.py @@ -0,0 +1,50 @@ +# Copyright 2017 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import unittest +from src.test.py.bazel import test_base + + +class BazelWindowsTest(test_base.TestBase): + + def testWindowsUnixRoot(self): + self.ScratchFile('WORKSPACE') + self.ScratchFile('foo/BUILD', ['cc_binary(name="x", srcs=["x.cc"])']) + self.ScratchFile('foo/x.cc', [ + '#include <stdio.h>', 'int main(int, char**) {' + ' printf("hello\\n");', ' return 0;', '}' + ]) + + exit_code, _, stderr = self.RunBazel( + ['--batch', 'build', '//foo:x', '--cpu=x64_windows_msys'], + env_remove={'BAZEL_SH'}) + self.assertEqual(exit_code, 2) + self.assertIn('\'BAZEL_SH\' environment variable is not set', + '\n'.join(stderr)) + + exit_code, _, stderr = self.RunBazel([ + '--batch', '--host_jvm_args=-Dbazel.windows_unix_root=', 'build', + '//foo:x', '--cpu=x64_windows_msys' + ]) + self.assertEqual(exit_code, 37) + self.assertIn('"bazel.windows_unix_root" JVM flag is not set', + '\n'.join(stderr)) + + exit_code, _, _ = self.RunBazel( + ['--batch', 'build', '//foo:x', '--cpu=x64_windows_msys']) + self.assertEqual(exit_code, 0) + + +if __name__ == '__main__': + unittest.main() diff --git a/src/test/py/bazel/empty_test.py b/src/test/py/bazel/empty_test.py new file mode 100644 index 0000000000..fe6a40a8ea --- /dev/null +++ b/src/test/py/bazel/empty_test.py @@ -0,0 +1,27 @@ +# Copyright 2017 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# Empty test for platforms that don't need to run a particular test. + +import unittest + + +class EmptyTest(unittest.TestCase): + + def testNothing(self): + pass + + +if __name__ == '__main__': + unittest.main() diff --git a/src/test/py/bazel/test_base.py b/src/test/py/bazel/test_base.py index 2a2b2e574e..7d0bba7385 100644 --- a/src/test/py/bazel/test_base.py +++ b/src/test/py/bazel/test_base.py @@ -117,7 +117,9 @@ class TestBase(unittest.TestCase): if not path: return abspath = self.Path(path) - if os.path.exists(abspath) and not os.path.isdir(abspath): + if os.path.exists(abspath): + if os.path.isdir(abspath): + return raise IOError('"%s" (%s) exists and is not a directory' % (path, abspath)) os.makedirs(abspath) @@ -145,11 +147,13 @@ class TestBase(unittest.TestCase): f.write(l) f.write('\n') - def RunBazel(self, args): + def RunBazel(self, args, env_remove=None): """Runs "bazel <args>", waits for it to exit. Args: args: [string]; flags to pass to bazel (e.g. ['--batch', 'build', '//x']) + env_remove: set(string); optional; environment variables to NOT pass to + Bazel Returns: (int, [string], [string]) tuple: exit code, stdout lines, stderr lines """ @@ -163,7 +167,7 @@ class TestBase(unittest.TestCase): stdout=stdout, stderr=stderr, cwd=self._tests_root, - env=self._BazelEnvMap()) + env=self._BazelEnvMap(env_remove)) exit_code = proc.wait() with open(self._stdout, 'r') as f: @@ -172,7 +176,7 @@ class TestBase(unittest.TestCase): stderr = [l.strip() for l in f.readlines()] return exit_code, stdout, stderr - def _BazelEnvMap(self): + def _BazelEnvMap(self, env_remove=None): """Returns the environment variable map to run Bazel.""" if TestBase.IsWindows(): result = [] @@ -190,12 +194,13 @@ class TestBase(unittest.TestCase): names.pop() os.path.walk('c:\\program files\\java\\', _Visit, result) + env = { 'SYSTEMROOT': TestBase.GetEnv('SYSTEMROOT'), # TODO(laszlocsomor): Let Bazel pass BAZEL_SH and JAVA_HOME to tests # and use those here instead of hardcoding paths. 'JAVA_HOME': 'c:\\program files\\java\\' + sorted(result)[-1], - 'BAZEL_SH': 'c:\\tools\\msys64\\usr\\bin\\bash.exe' + 'BAZEL_SH': 'c:\\tools\\msys64\\usr\\bin\\bash.exe', } else: env = {'HOME': os.path.join(self._temp, 'home')} @@ -206,6 +211,9 @@ class TestBase(unittest.TestCase): # that by checking for TEST_TMPDIR. env['TEST_TMPDIR'] = TestBase.GetEnv('TEST_TMPDIR') env['TMP'] = self._temp + if env_remove: + for e in env_remove: + del env[e] return env @staticmethod |