Fixed export_unity_package.py path issues on Windows.

Normalized all paths to use POSIX separators and fixed tests
to handle different file mode behavior on Windows.

Change-Id: I1ee231fc5b68024dee150809df97fe26818ecb9f

Author: Stewart Miles

Diff for: source/ExportUnityPackage/export_unity_package.py

@@ -541,13 +541,11 @@
 LINUX_SHARED_LIBRARY_EXTENSION = ".so"
 # Path relative to the "Assets" dir of native Linux plugin libraries.
 LINUX_SHARED_LIBRARY_PATH = re.compile(
-    r"^Plugins{sep}(x86|x86_64){sep}(.*{ext})".format(
-        sep=os.path.sep,
+    r"^Plugins/(x86|x86_64)/(.*{ext})".format(
         ext=LINUX_SHARED_LIBRARY_EXTENSION.replace(".", r"\.")))
 # Path components required for native desktop libraries.
 SHARED_LIBRARY_PATH = re.compile(
-    (r"(^|{sep})Plugins{sep}(x86|x86_64){sep}(.*{sep}|)[^{sep}]+"
-     r"\.(so|dll|bundle)$").format(sep=os.path.sep))
+    r"(^|/)Plugins/(x86|x86_64)/(.*/|)[^/]+\.(so|dll|bundle)$")
 # Prefix of the keywords to be added to UPM manifest to link to legacy manifest.
 UPM_KEYWORDS_MANIFEST_PREFIX = "vh-name:"
 # Everything in a Unity plugin - at the moment - lives under the Assets
@@ -570,6 +568,17 @@
   STR_OR_UNICODE = [str]
   unicode = str  # pylint: disable=redefined-builtin,invalid-name
 
+def posix_path(path):
+  """Convert path separators to POSIX style.
+
+  Args:
+    path: Path to convert.
+
+  Returns:
+    Path with POSIX separators, i.e / rather than \\.
+  """
+  return path.replace('\\', '/')
+
 
 class MissingGuidsError(Exception):
   """Raised when GUIDs are missing for input files in export_package().
@@ -638,7 +647,7 @@ def add_guid_and_path(self, guid, path):
       guid: GUID to add to this instance.
       path: Path associated with this GUID.
     """
-    self._paths_by_guid[guid].add(path)
+    self._paths_by_guid[guid].add(posix_path(path))
 
   def check_for_duplicates(self):
     """Check the set of GUIDs for duplicate paths.
@@ -960,7 +969,7 @@ def __init__(self, duplicate_guids_checker, guids_json,
       guid_map = safe_dict_get_value(guids_json, plugin_version,
                                      default_value={})
       for filename, guid in guid_map.items():
-        self.add_guid(filename, guid)
+        self.add_guid(posix_path(filename), guid)
 
       if plugin_version:
         # Aggregate guids for older versions of files.
@@ -983,6 +992,7 @@ def add_guid(self, path, guid):
       path: Path associated with the GUID.
       guid: GUID for the asset at the path.
     """
+    path = posix_path(path)
     self._guids_by_path[path] = guid
     self._duplicate_guids_checker.add_guid_and_path(guid, path)
 
@@ -1026,6 +1036,7 @@ def get_guid(self, path):
     Raises:
       MissingGuidsError: If the GUID isn't found.
     """
+    path = posix_path(path)
     guid = self._guids_by_path.get(path)
     if not guid:
       raise MissingGuidsError([path])
@@ -1108,7 +1119,7 @@ def version_handler_filename(filename, field_value_list):
     components.extend([VERSION_HANDLER_FIELD_SEPARATOR,
                        VERSION_HANDLER_FIELD_SEPARATOR.join(fields)])
   components.append(extension)
-  return "".join(components)
+  return posix_path("".join(components))
 
 
 class Asset(object):
@@ -1167,7 +1178,7 @@ def filename(self):
     Returns:
       Filename string.
     """
-    return self._filename
+    return posix_path(self._filename)
 
   @property
   def filename_absolute(self):
@@ -1176,7 +1187,7 @@ def filename_absolute(self):
     Returns:
       Filename string.
     """
-    return self._filename_absolute
+    return posix_path(self._filename_absolute)
 
   @property
   def filename_guid_lookup(self):
@@ -1185,7 +1196,7 @@ def filename_guid_lookup(self):
     Returns:
       Filename string.
     """
-    return self._filename_guid_lookup
+    return posix_path(self._filename_guid_lookup)
 
   @property
   def is_folder(self):
@@ -1239,7 +1250,7 @@ def disable_unsupported_platforms(importer_metadata, filename):
     Returns:
       Modified importer_metadata.
     """
-    filename = os.path.normpath(filename)
+    filename = posix_path(os.path.normpath(filename))
     is_shared_library = SHARED_LIBRARY_PATH.search(filename)
     if not is_shared_library:
       return importer_metadata
@@ -1557,7 +1568,8 @@ def write(self, output_dir, guid, timestamp=-1):
     # project.
     with open(os.path.join(output_asset_dir, "pathname"), "wt") as (
         pathname_file):
-      pathname_file.write(os.path.join(ASSETS_DIRECTORY, self.filename))
+      pathname_file.write(posix_path(os.path.join(ASSETS_DIRECTORY,
+                                                  self.filename)))
     return output_asset_dir
 
   def write_upm(self, output_dir, guid, timestamp=-1):
@@ -2331,7 +2343,8 @@ def write_manifest(self, output_dir, assets):
       os.makedirs(manifest_directory)
     with open(manifest_absolute_path, "wt") as manifest_file:
       manifest_file.write(
-          "%s\n" % "\n".join([os.path.join(ASSETS_DIRECTORY, asset.filename)
+          "%s\n" % "\n".join([posix_path(os.path.join(ASSETS_DIRECTORY,
+                                                      asset.filename))
                               for asset in Asset.sorted_by_filename(assets)]))
     # Retrieve a template manifest asset if it exists.
     manifest_asset = [asset for asset in assets
@@ -3295,7 +3308,7 @@ def copy_files_to_dir(colon_separated_input_output_filenames,
   copied_files = []
   for additional_file in colon_separated_input_output_filenames:
     additional_file_args = additional_file.split(":")
-    input_filename = additional_file_args[0]
+    input_filename = posix_path(additional_file_args[0])
 
     # Get the output filename.
     output_filename = input_filename
@@ -3306,11 +3319,11 @@ def copy_files_to_dir(colon_separated_input_output_filenames,
     # Remove the drive or root directory from the output filename.
     if os.path.normpath(output_filename).startswith(os.path.sep):
       output_filename = output_filename[len(os.path.sep):]
-    output_filename = os.path.join(output_dir, output_filename)
+    output_filename = posix_path(os.path.join(output_dir, output_filename))
 
     # Copy the file to the output directory.
     copy_and_set_rwx(input_filename, output_filename)
-    copied_files.append(output_filename)
+    copied_files.append(posix_path(output_filename))
   return copied_files
 
 

Diff for: source/ExportUnityPackage/export_unity_package_test.py

@@ -1289,9 +1289,10 @@ def test_write_manifest(self):
                        manifest_asset.filename)
       self.assertEqual("Foo/Bar/Test_version-1.2.3_manifest.txt",
                        manifest_asset.filename_guid_lookup)
-      manifest_absolute_path = os.path.join(
-          output_directory, "Foo/Bar/Test_version-1.2.3_manifest.txt")
-      self.assertEqual(manifest_absolute_path, manifest_asset.filename_absolute)
+      manifest_absolute_path = export_unity_package.posix_path(os.path.join(
+          output_directory, "Foo/Bar/Test_version-1.2.3_manifest.txt"))
+      self.assertEqual(manifest_absolute_path,
+                       manifest_asset.filename_absolute)
       self.assertFalse(manifest_asset.is_folder)
       self.assertEqual(self.expected_manifest_metadata,
                        manifest_asset.importer_metadata)
@@ -1355,7 +1356,8 @@ def test_write_upm_manifest(self):
       self.assertEqual("package.json", manifest_asset.filename)
       self.assertEqual("com.company.test/package.json",
                        manifest_asset.filename_guid_lookup)
-      manifest_absolute_path = os.path.join(output_directory, "package.json")
+      manifest_absolute_path = export_unity_package.posix_path(
+          os.path.join(output_directory, "package.json"))
       self.assertEqual(manifest_absolute_path, manifest_asset.filename_absolute)
       self.assertFalse(manifest_asset.is_folder)
       self.assertEqual(self.expected_upm_manifest_metadata,
@@ -2240,11 +2242,11 @@ def test_find_assets_in_multiple_directories(self):
         [os.path.join(self.assets_dir, "Firebase"),
          os.path.join(self.assets_dir, "PlayServicesResolver")])
     self.assertCountEqual(
-        [os.path.join(
-            self.assets_dir, "Firebase/Plugins/Firebase.Analytics.dll"),
-         os.path.join(
+        [export_unity_package.posix_path(os.path.join(
+            self.assets_dir, "Firebase/Plugins/Firebase.Analytics.dll")),
+         export_unity_package.posix_path(os.path.join(
              self.assets_dir,
-             "PlayServicesResolver/Editor/Google.VersionHandler.dll")],
+             "PlayServicesResolver/Editor/Google.VersionHandler.dll"))],
         [asset.filename_absolute for asset in found_assets])
 
   def test_find_assets_using_wildcard(self):
@@ -3245,6 +3247,10 @@ def setUp(self):
     super(FileOperationsTest, self).setUp()
     self.assets_dir = os.path.join(TEST_DATA_PATH, "Assets")
     self.temp_dir = os.path.join(FLAGS.test_tmpdir, "copy_temp")
+    self.expected_mode = stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR
+    if platform.system() == 'Windows':
+      # Windows doesn't support the executable mode so ignore it in tests.
+      self.expected_mode = self.expected_mode & ~stat.S_IXUSR
     os.makedirs(self.temp_dir)
 
   def tearDown(self):
@@ -3264,9 +3270,8 @@ def test_copy_and_set_rwx(self):
     export_unity_package.copy_and_set_rwx(source_path, target_path)
     self.assertTrue(os.path.exists(target_path))
     self.assertTrue(filecmp.cmp(source_path, target_path))
-    self.assertEqual(
-        stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR,
-        os.stat(target_path).st_mode & stat.S_IRWXU)
+    self.assertEqual(self.expected_mode,
+                     os.stat(target_path).st_mode & stat.S_IRWXU)
 
   def test_copy_and_set_rwx_new_dir(self):
     """Copy a file into a non-existent directory."""
@@ -3281,9 +3286,8 @@ def test_copy_and_set_rwx_new_dir(self):
     export_unity_package.copy_and_set_rwx(source_path, target_path)
     self.assertTrue(os.path.exists(target_path))
     self.assertTrue(filecmp.cmp(source_path, target_path))
-    self.assertEqual(
-        stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR,
-        os.stat(target_path).st_mode & stat.S_IRWXU)
+    self.assertEqual(self.expected_mode,
+                     os.stat(target_path).st_mode & stat.S_IRWXU)
 
   def test_copy_files_to_dir(self):
     """Test copying files into a directory using a variety of target paths."""
@@ -3324,11 +3328,12 @@ def test_copy_dir_to_dir(self):
     cmp_result = filecmp.dircmp(source_path, target_path)
     self.assertFalse(cmp_result.left_only or cmp_result.right_only or
                      cmp_result.diff_files)
+    # NOTE: Folders have the executable bit set on Windows.
     self.assertEqual(
         stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR,
         os.stat(os.path.join(target_path, "Editor")).st_mode & stat.S_IRWXU)
     self.assertEqual(
-        stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR,
+        self.expected_mode,
         (os.stat(os.path.join(target_path, "Editor.meta")).st_mode &
             stat.S_IRWXU))