diff --git a/git_config.py b/git_config.py index 778e81a..bc70d16 100644 --- a/git_config.py +++ b/git_config.py @@ -352,8 +352,8 @@ class GitConfig(object): Trace(': parsing %s', self.file) with open(self._json) as fd: return json.load(fd) - except (IOError, ValueError): - platform_utils.remove(self._json) + except (IOError, ValueErrorl): + platform_utils.remove(self._json, missing_ok=True) return None def _SaveJson(self, cache): @@ -361,8 +361,7 @@ class GitConfig(object): with open(self._json, 'w') as fd: json.dump(cache, fd, indent=2) except (IOError, TypeError): - if os.path.exists(self._json): - platform_utils.remove(self._json) + platform_utils.remove(self._json, missing_ok=True) def _ReadGit(self): """ diff --git a/manifest_xml.py b/manifest_xml.py index 135c91f..86f2020 100644 --- a/manifest_xml.py +++ b/manifest_xml.py @@ -270,8 +270,7 @@ class XmlManifest(object): self.Override(name) # Old versions of repo would generate symlinks we need to clean up. - if os.path.lexists(self.manifestFile): - platform_utils.remove(self.manifestFile) + platform_utils.remove(self.manifestFile, missing_ok=True) # This file is interpreted as if it existed inside the manifest repo. # That allows us to use with the relative file name. with open(self.manifestFile, 'w') as fp: diff --git a/platform_utils.py b/platform_utils.py index 5741f4d..0203249 100644 --- a/platform_utils.py +++ b/platform_utils.py @@ -127,28 +127,27 @@ def rename(src, dst): shutil.move(src, dst) -def remove(path): +def remove(path, missing_ok=False): """Remove (delete) the file path. This is a replacement for os.remove that allows deleting read-only files on Windows, with support for long paths and for deleting directory symbolic links. Availability: Unix, Windows.""" - if isWindows(): - longpath = _makelongpath(path) - try: - os.remove(longpath) - except OSError as e: - if e.errno == errno.EACCES: - os.chmod(longpath, stat.S_IWRITE) - # Directory symbolic links must be deleted with 'rmdir'. - if islink(longpath) and isdir(longpath): - os.rmdir(longpath) - else: - os.remove(longpath) + longpath = _makelongpath(path) if isWindows() else path + try: + os.remove(longpath) + except OSError as e: + if e.errno == errno.EACCES: + os.chmod(longpath, stat.S_IWRITE) + # Directory symbolic links must be deleted with 'rmdir'. + if islink(longpath) and isdir(longpath): + os.rmdir(longpath) else: - raise - else: - os.remove(path) + os.remove(longpath) + elif missing_ok and e.errno == errno.ENOENT: + pass + else: + raise def walk(top, topdown=True, onerror=None, followlinks=False): diff --git a/project.py b/project.py index fe88a50..634d88c 100644 --- a/project.py +++ b/project.py @@ -1182,10 +1182,8 @@ class Project(object): self._InitMRef() else: self._InitMirrorHead() - try: - platform_utils.remove(os.path.join(self.gitdir, 'FETCH_HEAD')) - except OSError: - pass + platform_utils.remove(os.path.join(self.gitdir, 'FETCH_HEAD'), + missing_ok=True) return True def PostRepoUpgrade(self): @@ -2307,15 +2305,12 @@ class Project(object): cmd.append('+refs/tags/*:refs/tags/*') ok = GitCommand(self, cmd, bare=True).Wait() == 0 - if os.path.exists(bundle_dst): - platform_utils.remove(bundle_dst) - if os.path.exists(bundle_tmp): - platform_utils.remove(bundle_tmp) + platform_utils.remove(bundle_dst, missing_ok=True) + platform_utils.remove(bundle_tmp, missing_ok=True) return ok def _FetchBundle(self, srcUrl, tmpPath, dstPath, quiet, verbose): - if os.path.exists(dstPath): - platform_utils.remove(dstPath) + platform_utils.remove(dstPath, missing_ok=True) cmd = ['curl', '--fail', '--output', tmpPath, '--netrc', '--location'] if quiet: @@ -2739,10 +2734,7 @@ class Project(object): # If the source file doesn't exist, ensure the destination # file doesn't either. if name in symlink_files and not os.path.lexists(src): - try: - platform_utils.remove(dst) - except OSError: - pass + platform_utils.remove(dst, missing_ok=True) except OSError as e: if e.errno == errno.EPERM: diff --git a/subcmds/sync.py b/subcmds/sync.py index c99b06c..3211cbb 100644 --- a/subcmds/sync.py +++ b/subcmds/sync.py @@ -767,13 +767,9 @@ later is required to fix a server side protocol bug. set(new_copyfile_paths)) for need_remove_file in need_remove_files: - try: - platform_utils.remove(need_remove_file) - except OSError as e: - if e.errno == errno.ENOENT: - # Try to remove the updated copyfile or linkfile. - # So, if the file is not exist, nothing need to do. - pass + # Try to remove the updated copyfile or linkfile. + # So, if the file is not exist, nothing need to do. + platform_utils.remove(need_remove_file, missing_ok=True) # Create copy-link-files.json, save dest path of "copyfile" and "linkfile". with open(copylinkfile_path, 'w', encoding='utf-8') as fp: @@ -1171,10 +1167,7 @@ class _FetchTimes(object): with open(self._path) as f: self._times = json.load(f) except (IOError, ValueError): - try: - platform_utils.remove(self._path) - except OSError: - pass + platform_utils.remove(self._path, missing_ok=True) self._times = {} def Save(self): @@ -1192,10 +1185,7 @@ class _FetchTimes(object): with open(self._path, 'w') as f: json.dump(self._times, f, indent=2) except (IOError, TypeError): - try: - platform_utils.remove(self._path) - except OSError: - pass + platform_utils.remove(self._path, missing_ok=True) # This is a replacement for xmlrpc.client.Transport using urllib2 # and supporting persistent-http[s]. It cannot change hosts from diff --git a/tests/test_platform_utils.py b/tests/test_platform_utils.py new file mode 100644 index 0000000..55b7805 --- /dev/null +++ b/tests/test_platform_utils.py @@ -0,0 +1,50 @@ +# Copyright 2021 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. + +"""Unittests for the platform_utils.py module.""" + +import os +import tempfile +import unittest + +import platform_utils + + +class RemoveTests(unittest.TestCase): + """Check remove() helper.""" + + def testMissingOk(self): + """Check missing_ok handling.""" + with tempfile.TemporaryDirectory() as tmpdir: + path = os.path.join(tmpdir, 'test') + + # Should not fail. + platform_utils.remove(path, missing_ok=True) + + # Should fail. + self.assertRaises(OSError, platform_utils.remove, path) + self.assertRaises(OSError, platform_utils.remove, path, missing_ok=False) + + # Should not fail if it exists. + open(path, 'w').close() + platform_utils.remove(path, missing_ok=True) + self.assertFalse(os.path.exists(path)) + + open(path, 'w').close() + platform_utils.remove(path) + self.assertFalse(os.path.exists(path)) + + open(path, 'w').close() + platform_utils.remove(path, missing_ok=False) + self.assertFalse(os.path.exists(path))