From 2e7029116204cf2d6f516e4514091f0b492bc689 Mon Sep 17 00:00:00 2001 From: Renaud Paquay Date: Tue, 1 Nov 2016 11:23:38 -0700 Subject: Make "git command" and "forall" work on Windows Python on Windows does not support non blocking file operations. To workaround this issue, we instead use Threads and a Queue to simulate non-blocking calls. This is happens only when running with the native Windows version of Python, meaning Linux and Cygwin are not affected by this change. Change-Id: I4ce23827b096c5138f67a85c721f58a12279bb6f --- platform_utils.py | 169 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 169 insertions(+) create mode 100644 platform_utils.py (limited to 'platform_utils.py') diff --git a/platform_utils.py b/platform_utils.py new file mode 100644 index 00000000..1c719b1d --- /dev/null +++ b/platform_utils.py @@ -0,0 +1,169 @@ +# +# Copyright (C) 2016 The Android Open Source Project +# +# 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 os +import platform +import select + +from Queue import Queue +from threading import Thread + + +def isWindows(): + """ Returns True when running with the native port of Python for Windows, + False when running on any other platform (including the Cygwin port of + Python). + """ + # Note: The cygwin port of Python returns "CYGWIN_NT_xxx" + return platform.system() == "Windows" + + +class FileDescriptorStreams(object): + """ Platform agnostic abstraction enabling non-blocking I/O over a + collection of file descriptors. This abstraction is required because + fctnl(os.O_NONBLOCK) is not supported on Windows. + """ + @classmethod + def create(cls): + """ Factory method: instantiates the concrete class according to the + current platform. + """ + if isWindows(): + return _FileDescriptorStreamsThreads() + else: + return _FileDescriptorStreamsNonBlocking() + + def __init__(self): + self.streams = [] + + def add(self, fd, dest, std_name): + """ Wraps an existing file descriptor as a stream. + """ + self.streams.append(self._create_stream(fd, dest, std_name)) + + def remove(self, stream): + """ Removes a stream, when done with it. + """ + self.streams.remove(stream) + + @property + def is_done(self): + """ Returns True when all streams have been processed. + """ + return len(self.streams) == 0 + + def select(self): + """ Returns the set of streams that have data available to read. + The returned streams each expose a read() and a close() method. + When done with a stream, call the remove(stream) method. + """ + raise NotImplementedError + + def _create_stream(fd, dest, std_name): + """ Creates a new stream wrapping an existing file descriptor. + """ + raise NotImplementedError + + +class _FileDescriptorStreamsNonBlocking(FileDescriptorStreams): + """ Implementation of FileDescriptorStreams for platforms that support + non blocking I/O. + """ + class Stream(object): + """ Encapsulates a file descriptor """ + def __init__(self, fd, dest, std_name): + self.fd = fd + self.dest = dest + self.std_name = std_name + self.set_non_blocking() + + def set_non_blocking(self): + import fcntl + flags = fcntl.fcntl(self.fd, fcntl.F_GETFL) + fcntl.fcntl(self.fd, fcntl.F_SETFL, flags | os.O_NONBLOCK) + + def fileno(self): + return self.fd.fileno() + + def read(self): + return self.fd.read(4096) + + def close(self): + self.fd.close() + + def _create_stream(self, fd, dest, std_name): + return self.Stream(fd, dest, std_name) + + def select(self): + ready_streams, _, _ = select.select(self.streams, [], []) + return ready_streams + + +class _FileDescriptorStreamsThreads(FileDescriptorStreams): + """ Implementation of FileDescriptorStreams for platforms that don't support + non blocking I/O. This implementation requires creating threads issuing + blocking read operations on file descriptors. + """ + def __init__(self): + super(_FileDescriptorStreamsThreads, self).__init__() + # The queue is shared accross all threads so we can simulate the + # behavior of the select() function + self.queue = Queue(10) # Limit incoming data from streams + + def _create_stream(self, fd, dest, std_name): + return self.Stream(fd, dest, std_name, self.queue) + + def select(self): + # Return only one stream at a time, as it is the most straighforward + # thing to do and it is compatible with the select() function. + item = self.queue.get() + stream = item.stream + stream.data = item.data + return [stream] + + class QueueItem(object): + """ Item put in the shared queue """ + def __init__(self, stream, data): + self.stream = stream + self.data = data + + class Stream(object): + """ Encapsulates a file descriptor """ + def __init__(self, fd, dest, std_name, queue): + self.fd = fd + self.dest = dest + self.std_name = std_name + self.queue = queue + self.data = None + self.thread = Thread(target=self.read_to_queue) + self.thread.daemon = True + self.thread.start() + + def close(self): + self.fd.close() + + def read(self): + data = self.data + self.data = None + return data + + def read_to_queue(self): + """ The thread function: reads everything from the file descriptor into + the shared queue and terminates when reaching EOF. + """ + for line in iter(self.fd.readline, b''): + self.queue.put(_FileDescriptorStreamsThreads.QueueItem(self, line)) + self.fd.close() + self.queue.put(_FileDescriptorStreamsThreads.QueueItem(self, None)) -- cgit v1.2.3-54-g00ecf From d5cec5e752821ca2710101b626b3a3ca07fdb7f8 Mon Sep 17 00:00:00 2001 From: Renaud Paquay Date: Tue, 1 Nov 2016 11:24:03 -0700 Subject: Add support for creating symbolic links on Windows Replace all calls to os.symlink with platform_utils.symlink. The Windows implementation calls into the CreateSymbolicLinkW Win32 API, as os.symlink is not supported. Separate the Win32 API definitions into a separate module platform_utils_win32 for clarity. Change-Id: I0714c598664c2df93383734e609d948692c17ec5 --- manifest_xml.py | 3 ++- platform_utils.py | 43 +++++++++++++++++++++++++++++++++ platform_utils_win32.py | 63 +++++++++++++++++++++++++++++++++++++++++++++++++ project.py | 9 ++++--- 4 files changed, 114 insertions(+), 4 deletions(-) create mode 100644 platform_utils_win32.py (limited to 'platform_utils.py') diff --git a/manifest_xml.py b/manifest_xml.py index 55d25a79..05651c6c 100644 --- a/manifest_xml.py +++ b/manifest_xml.py @@ -32,6 +32,7 @@ else: import gitc_utils from git_config import GitConfig from git_refs import R_HEADS, HEAD +import platform_utils from project import RemoteSpec, Project, MetaProject from error import ManifestParseError, ManifestInvalidRevisionError @@ -166,7 +167,7 @@ class XmlManifest(object): try: if os.path.lexists(self.manifestFile): os.remove(self.manifestFile) - os.symlink(os.path.join('manifests', name), self.manifestFile) + platform_utils.symlink(os.path.join('manifests', name), self.manifestFile) except OSError as e: raise ManifestParseError('cannot link manifest %s: %s' % (name, str(e))) diff --git a/platform_utils.py b/platform_utils.py index 1c719b1d..f4dfa0b1 100644 --- a/platform_utils.py +++ b/platform_utils.py @@ -167,3 +167,46 @@ class _FileDescriptorStreamsThreads(FileDescriptorStreams): self.queue.put(_FileDescriptorStreamsThreads.QueueItem(self, line)) self.fd.close() self.queue.put(_FileDescriptorStreamsThreads.QueueItem(self, None)) + + +def symlink(source, link_name): + """Creates a symbolic link pointing to source named link_name. + Note: On Windows, source must exist on disk, as the implementation needs + to know whether to create a "File" or a "Directory" symbolic link. + """ + if isWindows(): + import platform_utils_win32 + source = _validate_winpath(source) + link_name = _validate_winpath(link_name) + target = os.path.join(os.path.dirname(link_name), source) + if os.path.isdir(target): + platform_utils_win32.create_dirsymlink(source, link_name) + else: + platform_utils_win32.create_filesymlink(source, link_name) + else: + return os.symlink(source, link_name) + + +def _validate_winpath(path): + path = os.path.normpath(path) + if _winpath_is_valid(path): + return path + raise ValueError("Path \"%s\" must be a relative path or an absolute " + "path starting with a drive letter".format(path)) + + +def _winpath_is_valid(path): + """Windows only: returns True if path is relative (e.g. ".\\foo") or is + absolute including a drive letter (e.g. "c:\\foo"). Returns False if path + is ambiguous (e.g. "x:foo" or "\\foo"). + """ + assert isWindows() + path = os.path.normpath(path) + drive, tail = os.path.splitdrive(path) + if tail: + if not drive: + return tail[0] != os.sep # "\\foo" is invalid + else: + return tail[0] == os.sep # "x:foo" is invalid + else: + return not drive # "x:" is invalid diff --git a/platform_utils_win32.py b/platform_utils_win32.py new file mode 100644 index 00000000..02fb013a --- /dev/null +++ b/platform_utils_win32.py @@ -0,0 +1,63 @@ +# +# Copyright (C) 2016 The Android Open Source Project +# +# 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 errno + +from ctypes import WinDLL, get_last_error, FormatError, WinError +from ctypes.wintypes import BOOL, LPCWSTR, DWORD + +kernel32 = WinDLL('kernel32', use_last_error=True) + +# Win32 error codes +ERROR_SUCCESS = 0 +ERROR_PRIVILEGE_NOT_HELD = 1314 + +# Win32 API entry points +CreateSymbolicLinkW = kernel32.CreateSymbolicLinkW +CreateSymbolicLinkW.restype = BOOL +CreateSymbolicLinkW.argtypes = (LPCWSTR, # lpSymlinkFileName In + LPCWSTR, # lpTargetFileName In + DWORD) # dwFlags In + +# Symbolic link creation flags +SYMBOLIC_LINK_FLAG_FILE = 0x00 +SYMBOLIC_LINK_FLAG_DIRECTORY = 0x01 + + +def create_filesymlink(source, link_name): + """Creates a Windows file symbolic link source pointing to link_name.""" + _create_symlink(source, link_name, SYMBOLIC_LINK_FLAG_FILE) + + +def create_dirsymlink(source, link_name): + """Creates a Windows directory symbolic link source pointing to link_name. + """ + _create_symlink(source, link_name, SYMBOLIC_LINK_FLAG_DIRECTORY) + + +def _create_symlink(source, link_name, dwFlags): + # Note: Win32 documentation for CreateSymbolicLink is incorrect. + # On success, the function returns "1". + # On error, the function returns some random value (e.g. 1280). + # The best bet seems to use "GetLastError" and check for error/success. + CreateSymbolicLinkW(link_name, source, dwFlags) + code = get_last_error() + if code != ERROR_SUCCESS: + error_desc = FormatError(code).strip() + if code == ERROR_PRIVILEGE_NOT_HELD: + raise OSError(errno.EPERM, error_desc, link_name) + error_desc = 'Error creating symbolic link %s: %s'.format( + link_name, error_desc) + raise WinError(code, error_desc) diff --git a/project.py b/project.py index 269fd7e5..de5c7915 100644 --- a/project.py +++ b/project.py @@ -35,6 +35,7 @@ from git_config import GitConfig, IsId, GetSchemeFromUrl, GetUrlCookieFile, \ from error import GitError, HookError, UploadError, DownloadError from error import ManifestInvalidRevisionError from error import NoManifestException +import platform_utils from trace import IsTrace, Trace from git_refs import GitRefs, HEAD, R_HEADS, R_TAGS, R_PUB, R_M @@ -277,7 +278,7 @@ class _LinkFile(object): dest_dir = os.path.dirname(absDest) if not os.path.isdir(dest_dir): os.makedirs(dest_dir) - os.symlink(relSrc, absDest) + platform_utils.symlink(relSrc, absDest) except IOError: _error('Cannot link file %s to %s', relSrc, absDest) @@ -2379,7 +2380,8 @@ class Project(object): self.relpath, name) continue try: - os.symlink(os.path.relpath(stock_hook, os.path.dirname(dst)), dst) + platform_utils.symlink( + os.path.relpath(stock_hook, os.path.dirname(dst)), dst) except OSError as e: if e.errno == errno.EPERM: raise GitError('filesystem must support symlinks') @@ -2478,7 +2480,8 @@ class Project(object): os.makedirs(src) if name in to_symlink: - os.symlink(os.path.relpath(src, os.path.dirname(dst)), dst) + platform_utils.symlink( + os.path.relpath(src, os.path.dirname(dst)), dst) elif copy_all and not os.path.islink(dst): if os.path.isdir(src): shutil.copytree(src, dst) -- cgit v1.2.3-54-g00ecf From a65adf74f990eeac0d90011476376c7239cb7af5 Mon Sep 17 00:00:00 2001 From: Renaud Paquay Date: Thu, 3 Nov 2016 10:37:53 -0700 Subject: Workaround shutil.rmtree limitation on Windows By default, shutil.rmtree raises an exception when deleting readonly files on Windows. Replace all shutil.rmtree with platform_utils.rmtree, which adds an error handler to make files read-write when they can't be deleted. Change-Id: I9cfea9a7b3703fb16a82cf69331540c2c179ed53 --- platform_utils.py | 15 +++++++++++++++ project.py | 12 ++++++------ subcmds/gitc_delete.py | 4 ++-- subcmds/init.py | 4 ++-- subcmds/sync.py | 4 ++-- 5 files changed, 27 insertions(+), 12 deletions(-) (limited to 'platform_utils.py') diff --git a/platform_utils.py b/platform_utils.py index f4dfa0b1..4417c5a3 100644 --- a/platform_utils.py +++ b/platform_utils.py @@ -16,6 +16,8 @@ import os import platform import select +import shutil +import stat from Queue import Queue from threading import Thread @@ -210,3 +212,16 @@ def _winpath_is_valid(path): return tail[0] == os.sep # "x:foo" is invalid else: return not drive # "x:" is invalid + + +def rmtree(path): + if isWindows(): + shutil.rmtree(path, onerror=handle_rmtree_error) + else: + shutil.rmtree(path) + + +def handle_rmtree_error(function, path, excinfo): + # Allow deleting read-only files + os.chmod(path, stat.S_IWRITE) + function(path) diff --git a/project.py b/project.py index de5c7915..ba18337b 100644 --- a/project.py +++ b/project.py @@ -2299,10 +2299,10 @@ class Project(object): print("Retrying clone after deleting %s" % self.gitdir, file=sys.stderr) try: - shutil.rmtree(os.path.realpath(self.gitdir)) + platform_utils.rmtree(os.path.realpath(self.gitdir)) if self.worktree and os.path.exists(os.path.realpath (self.worktree)): - shutil.rmtree(os.path.realpath(self.worktree)) + platform_utils.rmtree(os.path.realpath(self.worktree)) return self._InitGitDir(mirror_git=mirror_git, force_sync=False) except: raise e @@ -2344,9 +2344,9 @@ class Project(object): self.config.SetString('core.bare', None) except Exception: if init_obj_dir and os.path.exists(self.objdir): - shutil.rmtree(self.objdir) + platform_utils.rmtree(self.objdir) if init_git_dir and os.path.exists(self.gitdir): - shutil.rmtree(self.gitdir) + platform_utils.rmtree(self.gitdir) raise def _UpdateHooks(self): @@ -2516,7 +2516,7 @@ class Project(object): except GitError as e: if force_sync: try: - shutil.rmtree(dotgit) + platform_utils.rmtree(dotgit) return self._InitWorkTree(force_sync=False, submodules=submodules) except: raise e @@ -2536,7 +2536,7 @@ class Project(object): self._CopyAndLinkFiles() except Exception: if init_dotgit: - shutil.rmtree(dotgit) + platform_utils.rmtree(dotgit) raise def _gitdir_path(self, path): diff --git a/subcmds/gitc_delete.py b/subcmds/gitc_delete.py index 19caac5a..54f62f46 100644 --- a/subcmds/gitc_delete.py +++ b/subcmds/gitc_delete.py @@ -14,10 +14,10 @@ # limitations under the License. from __future__ import print_function -import shutil import sys from command import Command, GitcClientCommand +import platform_utils from pyversion import is_python3 if not is_python3(): @@ -50,4 +50,4 @@ and all locally downloaded sources. if not response == 'yes': print('Response was not "yes"\n Exiting...') sys.exit(1) - shutil.rmtree(self.gitc_manifest.gitc_client_dir) + platform_utils.rmtree(self.gitc_manifest.gitc_client_dir) diff --git a/subcmds/init.py b/subcmds/init.py index 65dfd1fd..e6470916 100644 --- a/subcmds/init.py +++ b/subcmds/init.py @@ -17,7 +17,6 @@ from __future__ import print_function import os import platform import re -import shutil import sys from pyversion import is_python3 @@ -35,6 +34,7 @@ from error import ManifestParseError from project import SyncBuffer from git_config import GitConfig from git_command import git_require, MIN_GIT_VERSION +import platform_utils class Init(InteractiveCommand, MirrorSafeCommand): common = True @@ -252,7 +252,7 @@ to update the working directory files. # Better delete the manifest git dir if we created it; otherwise next # time (when user fixes problems) we won't go through the "is_new" logic. if is_new: - shutil.rmtree(m.gitdir) + platform_utils.rmtree(m.gitdir) sys.exit(1) if opt.manifest_branch: diff --git a/subcmds/sync.py b/subcmds/sync.py index ef023274..797fc403 100644 --- a/subcmds/sync.py +++ b/subcmds/sync.py @@ -19,7 +19,6 @@ import netrc from optparse import SUPPRESS_HELP import os import re -import shutil import socket import subprocess import sys @@ -73,6 +72,7 @@ from project import Project from project import RemoteSpec from command import Command, MirrorSafeCommand from error import RepoChangedException, GitError, ManifestParseError +import platform_utils from project import SyncBuffer from progress import Progress from wrapper import Wrapper @@ -473,7 +473,7 @@ later is required to fix a server side protocol bug. # working git repository around. There shouldn't be any git projects here, # so rmtree works. try: - shutil.rmtree(os.path.join(path, '.git')) + platform_utils.rmtree(os.path.join(path, '.git')) except OSError: print('Failed to remove %s' % os.path.join(path, '.git'), file=sys.stderr) print('error: Failed to delete obsolete path %s' % path, file=sys.stderr) -- cgit v1.2.3-54-g00ecf From ad1abcb556bfff2744928a8f29d216aee43fdbe3 Mon Sep 17 00:00:00 2001 From: Renaud Paquay Date: Tue, 1 Nov 2016 11:34:55 -0700 Subject: Port os.rename calls to work on Windows os.rename fails on Windows if the destination exists, so replace os.rename to platform_utils.rename which handles the platform differences. Change-Id: I15a86f10f65eedee5b003b80f88a0c28a3e1aa48 --- platform_utils.py | 17 +++++++++++++++++ project.py | 4 ++-- 2 files changed, 19 insertions(+), 2 deletions(-) (limited to 'platform_utils.py') diff --git a/platform_utils.py b/platform_utils.py index 4417c5a3..e0fa9dcc 100644 --- a/platform_utils.py +++ b/platform_utils.py @@ -13,6 +13,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import errno import os import platform import select @@ -225,3 +226,19 @@ def handle_rmtree_error(function, path, excinfo): # Allow deleting read-only files os.chmod(path, stat.S_IWRITE) function(path) + + +def rename(src, dst): + if isWindows(): + # On Windows, rename fails if destination exists, see + # https://docs.python.org/2/library/os.html#os.rename + try: + os.rename(src, dst) + except OSError as e: + if e.errno == errno.EEXIST: + os.remove(dst) + os.rename(src, dst) + else: + raise + else: + os.rename(src, dst) diff --git a/project.py b/project.py index ba18337b..e8de4842 100644 --- a/project.py +++ b/project.py @@ -63,7 +63,7 @@ def _lwrite(path, content): fd.close() try: - os.rename(lock, path) + platform_utils.rename(lock, path) except OSError: os.remove(lock) raise @@ -2198,7 +2198,7 @@ class Project(object): if os.path.exists(tmpPath): if curlret == 0 and self._IsValidBundle(tmpPath, quiet): - os.rename(tmpPath, dstPath) + platform_utils.rename(tmpPath, dstPath) return True else: os.remove(tmpPath) -- cgit v1.2.3-54-g00ecf