From 4904e1bd9cc5d11035e2664a9fdcce658dd22a3a Mon Sep 17 00:00:00 2001 From: Matt Glazar Date: Thu, 23 May 2019 15:38:29 -0700 Subject: [PATCH] Reuse old build dir on Windows Summary: getdeps.py's find_existing_win32_subst_for_path function tries to reuse an existing build directory alias. (On Windows, the build directory is aliased to a drive letter to shorten paths.) If this function does not find and existing build directory alias, getdeps.py invalidates many of its caches. On my Windows machine, find_existing_win32_subst_for_path always fails, so all of my builds are super slow. This happens because find_existing_win32_subst_for_path is given a path with a lower-case drive letter ("c:\users\..."), but the subst command returns paths with an upper-case drive letter ("C:\users\..."). When comparing paths, use ntpath.normpath. This makes the comparison case-insensitive for drive letters. (It also makes the comparison case-insensitive for other path components.) On Linux and macOS, this diff should not change behavior. Reviewed By: wez Differential Revision: D15466096 fbshipit-source-id: 1669aa0a6eaeb6012154f3a9e24eba3f835262c6 --- build/fbcode_builder/getdeps/buildopts.py | 18 ++-- .../getdeps/test/scratch_test.py | 83 +++++++++++++++++++ 2 files changed, 95 insertions(+), 6 deletions(-) create mode 100644 build/fbcode_builder/getdeps/test/scratch_test.py diff --git a/build/fbcode_builder/getdeps/buildopts.py b/build/fbcode_builder/getdeps/buildopts.py index 2bc73d9a1..70d3662b2 100644 --- a/build/fbcode_builder/getdeps/buildopts.py +++ b/build/fbcode_builder/getdeps/buildopts.py @@ -10,6 +10,7 @@ from __future__ import absolute_import, division, print_function, unicode_litera import base64 import errno import hashlib +import ntpath import os import subprocess import tempfile @@ -178,11 +179,14 @@ def list_win32_subst_letters(): return mapping -def find_existing_win32_subst_for_path(path): - path = os.path.normpath(path) - mapping = list_win32_subst_letters() - for letter, target in mapping.items(): - if target == path: +def find_existing_win32_subst_for_path( + path, # type: str + subst_mapping, # type: typing.Mapping[str, str] +): + # type: (...) -> typing.Optional[str] + path = ntpath.normcase(ntpath.normpath(path)) + for letter, target in subst_mapping.items(): + if ntpath.normcase(target) == path: return letter return None @@ -210,7 +214,9 @@ def find_unused_drive_letter(): def create_subst_path(path): for _attempt in range(0, 24): - drive = find_existing_win32_subst_for_path(path) + drive = find_existing_win32_subst_for_path( + path, subst_mapping=list_win32_subst_letters() + ) if drive: return drive available = find_unused_drive_letter() diff --git a/build/fbcode_builder/getdeps/test/scratch_test.py b/build/fbcode_builder/getdeps/test/scratch_test.py new file mode 100644 index 000000000..fd8f4819e --- /dev/null +++ b/build/fbcode_builder/getdeps/test/scratch_test.py @@ -0,0 +1,83 @@ +#!/usr/bin/env python +# Copyright (c) 2019-present, Facebook, Inc. +# All rights reserved. +# +# This source code is licensed under the BSD-style license found in the +# LICENSE file in the root directory of this source tree. An additional grant +# of patent rights can be found in the PATENTS file in the same directory. + +from __future__ import absolute_import, division, print_function, unicode_literals + +import unittest + +from ..buildopts import find_existing_win32_subst_for_path + + +class Win32SubstTest(unittest.TestCase): + def test_no_existing_subst(self): + self.assertIsNone( + find_existing_win32_subst_for_path( + r"C:\users\alice\appdata\local\temp\fbcode_builder_getdeps", + subst_mapping={}, + ) + ) + self.assertIsNone( + find_existing_win32_subst_for_path( + r"C:\users\alice\appdata\local\temp\fbcode_builder_getdeps", + subst_mapping={"X:\\": r"C:\users\alice\appdata\local\temp\other"}, + ) + ) + + def test_exact_match_returns_drive_path(self): + self.assertEqual( + find_existing_win32_subst_for_path( + r"C:\temp\fbcode_builder_getdeps", + subst_mapping={"X:\\": r"C:\temp\fbcode_builder_getdeps"}, + ), + "X:\\", + ) + self.assertEqual( + find_existing_win32_subst_for_path( + r"C:/temp/fbcode_builder_getdeps", + subst_mapping={"X:\\": r"C:/temp/fbcode_builder_getdeps"}, + ), + "X:\\", + ) + + def test_multiple_exact_matches_returns_arbitrary_drive_path(self): + self.assertIn( + find_existing_win32_subst_for_path( + r"C:\temp\fbcode_builder_getdeps", + subst_mapping={ + "X:\\": r"C:\temp\fbcode_builder_getdeps", + "Y:\\": r"C:\temp\fbcode_builder_getdeps", + "Z:\\": r"C:\temp\fbcode_builder_getdeps", + }, + ), + ("X:\\", "Y:\\", "Z:\\"), + ) + + def test_drive_letter_is_case_insensitive(self): + self.assertEqual( + find_existing_win32_subst_for_path( + r"C:\temp\fbcode_builder_getdeps", + subst_mapping={"X:\\": r"c:\temp\fbcode_builder_getdeps"}, + ), + "X:\\", + ) + + def test_path_components_are_case_insensitive(self): + self.assertEqual( + find_existing_win32_subst_for_path( + r"C:\TEMP\FBCODE_builder_getdeps", + subst_mapping={"X:\\": r"C:\temp\fbcode_builder_getdeps"}, + ), + "X:\\", + ) + self.assertEqual( + find_existing_win32_subst_for_path( + r"C:\temp\fbcode_builder_getdeps", + subst_mapping={"X:\\": r"C:\TEMP\FBCODE_builder_getdeps"}, + ), + "X:\\", + )