fix: 'Error copying file' error occured when an an external cab was m…

…anually placed into the source directory and the source directory was the same directory as the output directory fixes #169

* chore: make test output display more info when compare fails

* fix: 'Error copying file' error occured when an an external cab was manually placed into the source directory and the source directory was the same directory as the output directory

fixes #169

* chore: test for #169

* chore: cleanning up code

src/LessMsi.Core/Msi/Wixtracts.cs

@@ -328,7 +328,7 @@ public static void ExtractFiles(Path msi, string outputDir, MsiFile[] filesToExt
                             LessIO.Path destName = LessIO.Path.Combine(targetDirectoryForFile, entry.LongFileName);
 					        if (FileSystem.Exists(destName))
 					        {
-						        Debug.Fail("output file already exists. We'll make it unique, but this is probably a strange msi or a bug in this program.");
+						        Debug.Fail(string.Format("output file '{0}' already exists. We'll make it unique, but this is probably a strange msi or a bug in this program.", destName));
 						        //make unique
 						        // ReSharper disable HeuristicUnreachableCode
 						        Trace.WriteLine(string.Concat("Duplicate file found \'", destName, "\'"));
@@ -352,9 +352,16 @@ public static void ExtractFiles(Path msi, string outputDir, MsiFile[] filesToExt
 			        foreach (var decomp in cabDecompressors)
 			        {
 				        decomp.Close(false);
-				        DeleteFileForcefully(new Path(decomp.LocalFilePath));
 			        }
-		        }
+                    // also delete any cabs we copied:
+                    foreach (var cabInf in cabInfos)
+                    {
+                        if (cabInf.DoesNeedDeleted)
+                        {
+                            DeleteFileForcefully(new Path(cabInf.LocalCabFile));
+                        }
+                    }
+                }
 	        }
 	        finally
 	        {
@@ -487,6 +494,7 @@ private static List<CabInfo> CabsFromMsiToDisk(Path msi, Database msidb, string
 						if (!string.IsNullOrEmpty(cabSourceName))
 						{
 							bool extract = false;
+                            bool doDeleteLater = true;
                             // NOTE: If the cabinet name is preceded by the number sign, the cabinet is stored as a data stream inside the package. https://docs.microsoft.com/en-us/windows/win32/msi/cabinet
                             if (cabSourceName.StartsWith("#"))
 							{
@@ -506,13 +514,18 @@ private static List<CabInfo> CabsFromMsiToDisk(Path msi, Database msidb, string
                                 {
                                     throw ExternalCabNotFoundException.CreateFromCabPath(cabSourceName, msi.Parent.FullPathString);
                                 }
-								FileSystem.Copy(originalCabFile, localCabFile);
+                                // In cases like https://github.com/activescott/lessmsi/issues/169 the cab file was originally an embedded cab but the user extracted the cab into the same directory as the MSI manually, so it may already be there
+                                if (!originalCabFile.Equals(localCabFile)) {
+                                    FileSystem.Copy(originalCabFile, localCabFile);
+                                } else {
+                                    doDeleteLater = false;
+                                }
 							}
 							/* http://code.google.com/p/lessmsi/issues/detail?id=1
 								* apparently in some cases a file spans multiple CABs (VBRuntime.msi) so due to that we have get all CAB files out of the MSI and then begin extraction. Then after we extract everything out of all CAbs we need to release the CAB extractors and delete temp files.
 								* Thanks to Christopher Hamburg for explaining this!
 							*/
-							var c = new CabInfo(localCabFile.PathString, cabSourceName);
+							var c = new CabInfo(localCabFile.PathString, cabSourceName, doDeleteLater);
 							localCabFiles.Add(c);
 						}
 					}
@@ -527,15 +540,20 @@ class CabInfo
 			/// Name of the cab in the MSI.
 			/// </summary>
 			public string CabSourceName { get; set; }
-			/// <summary>
-			/// Path of the CAB on local disk after we pop it out of the msi.
+            /// <summary>
+			/// True if the cab needs deleted later.
 			/// </summary>
-			public string LocalCabFile { get; set; } //TODO: Make LocalCabFile use LessIO.Path
+			public bool DoesNeedDeleted { get; }
+            /// <summary>
+            /// Path of the CAB on local disk after we pop it out of the msi.
+            /// </summary>
+            public string LocalCabFile { get; set; } //TODO: Make LocalCabFile use LessIO.Path
 
-			public CabInfo(string localCabFile, string cabSourceName)
+			public CabInfo(string localCabFile, string cabSourceName, bool doesNeedDeleted)
 			{
 				LocalCabFile = localCabFile;
 				CabSourceName = cabSourceName;
+                DoesNeedDeleted = doesNeedDeleted;
 			}
 		}
 

src/Lessmsi.Tests/FileEntryGraph.cs

@@ -174,22 +174,23 @@ private static string SerializeDate(DateTime dateTime)
 
 	    public static bool CompareEntries(FileEntryGraph a, FileEntryGraph b, out string errorMessage)
         {
+            errorMessage = "";
+            bool suceeded = true;
             if (a.Entries.Count != b.Entries.Count)
             {
                 errorMessage = string.Format("Entries for '{0}' and '{1}' have a different number of file entries ({2}, {3} respectively).", a.ForFileName, b.ForFileName, a.Entries.Count, b.Entries.Count);
-                return false;
+                suceeded = false;
             }
 
             for (int i = 0; i < Math.Max(a.Entries.Count, b.Entries.Count); i++)
             {
                 if (!a.Entries[i].Equals(b.Entries[i]))
                 {
-                    errorMessage = string.Format("'{0}'!='{1}' at index '{2}'.", a.Entries[i].Path, b.Entries[i].Path, i);
-                    return false;
+                    errorMessage += string.Format("'{0}'!='{1}' at index '{2}'.", a.Entries[i].Path, b.Entries[i].Path, i);
+                    suceeded = false;
                 }
             }
-            errorMessage = "";
-            return true;
+            return suceeded;
         }
     }
 }

src/Lessmsi.Tests/MiscTests.cs

@@ -1,4 +1,5 @@
 using Xunit;
+using LessIO;
 
 namespace LessMsi.Tests
 {
@@ -59,6 +60,37 @@ public void ExtractFromExternalCab()
 			ExtractAndCompareToMaster("msi_with_external_cab.msi");
 		}
 
+		/// <summary>
+		/// This test is for github issue 169: https://github.com/activescott/lessmsi/issues/169
+		/// </summary>
+		[Fact]
+		public void ExtractFromExternalCabWithSourceDirAndOutputDirSame()
+		{
+			var msiFileName = "vcredist.msi";
+            var cabFileName = "vcredis1.cab";
+            // put the msi and cab into the output directory (as this is all about having the source dir and output dir be the same):
+            var outputDir = GetTestOutputDir(Path.Empty, msiFileName);
+            if (FileSystem.Exists(outputDir))
+            {
+                FileSystem.RemoveDirectory(outputDir, true);
+            }
+            FileSystem.CreateDirectory(outputDir);
+            var msiFileOutputDir = Path.Combine(outputDir, msiFileName);
+            FileSystem.Copy(GetMsiTestFile(msiFileName), msiFileOutputDir);
+			FileSystem.Copy(GetMsiTestFile(cabFileName), Path.Combine(outputDir, cabFileName));
+
+            // run test normally:
+            LessMsi.Msi.Wixtracts.ExtractFiles(msiFileOutputDir, outputDir.PathString);
+            //  build actual file entries extracted
+            var actualFileEntries = FileEntryGraph.GetActualEntries(outputDir.PathString, msiFileName);
+            // this test has the original msi and cab as the first two entries and their times change. so we drop them here:
+            actualFileEntries.Entries.RemoveRange(0, 2);
+            // dump to actual dir (for debugging and updating tests)
+            actualFileEntries.Save(GetActualOutputFile(msiFileName));
+            var expectedEntries = GetExpectedEntriesForMsi(msiFileName);
+			AssertAreEqual(expectedEntries, actualFileEntries);
+		}
+
 		/// <summary>
 		/// This one demonstrates a problem were paths are screwed up. 
 		/// Note that the output path ends up being SourceDir\SlikSvn\bin\Windows\winsxs\... and it should be just \windows\winsxs\...

src/Lessmsi.Tests/TestBase.cs

@@ -23,7 +23,7 @@ protected static void AssertAreEqual(FileEntryGraph expected, FileEntryGraph act
             string msg;
             if (!FileEntryGraph.CompareEntries(expected, actual, out msg))
             {
-                throw new Exception("FileEntryGraph entries are not the equal");
+                throw new Exception(string.Format("FileEntryGraph entries are not the equal: {0}", msg));
             }
         }
 
@@ -37,18 +37,24 @@ protected FileEntryGraph ExtractFilesFromMsi(string msiFileName, string[] fileNa
             return ExtractFilesFromMsi(msiFileName, fileNamesToExtractOrNull, new Path(outputDir), true);
         }
 
+        protected FileEntryGraph ExtractFilesFromMsi(string msiFileName, string[] fileNamesToExtractOrNull, Path outputDir, bool returnFileEntryGraph)
+        {
+            return ExtractFilesFromMsi(msiFileName, fileNamesToExtractOrNull, outputDir, true, true);
+        }
+
         /// <summary>
         /// Extracts some or all of the files from the specified MSI and returns a <see cref="FileEntryGraph"/> representing the files that were extracted.
         /// </summary>
         /// <param name="msiFileName">The msi file to extract or null to extract all files.</param>
         /// <param name="fileNamesToExtractOrNull">The files to extract from the MSI or null to extract all files.</param>
         /// <param name="outputDir">A relative directory to extract output to or an empty string to use the default output directory.</param>
         /// <param name="skipReturningFileEntryGraph">True to return the <see cref="FileEntryGraph"/>. Otherwise null will be returned.</param>
-        protected FileEntryGraph ExtractFilesFromMsi(string msiFileName, string[] fileNamesToExtractOrNull, Path outputDir, bool returnFileEntryGraph)
+        /// <param name="cleanOutputDirectoryBeforeExtracting">True to delete the output directory before extracting.</param>
+        protected FileEntryGraph ExtractFilesFromMsi(string msiFileName, string[] fileNamesToExtractOrNull, Path outputDir, bool returnFileEntryGraph, bool cleanOutputDirectoryBeforeExtracting)
         {
             outputDir = GetTestOutputDir(outputDir, msiFileName);
 
-            if (FileSystem.Exists(outputDir))
+            if (cleanOutputDirectoryBeforeExtracting && FileSystem.Exists(outputDir))
             {
                 FileSystem.RemoveDirectory(outputDir, true);
             }
@@ -210,7 +216,7 @@ protected FileEntryGraph GetExpectedEntriesForMsi(string forMsi)
             return FileEntryGraph.Load(GetExpectedOutputFile(forMsi), forMsi);
         }
 
-        private Path GetMsiTestFile(string msiFileName)
+        protected Path GetMsiTestFile(string msiFileName)
         {
             return Path.Combine(AppPath, "TestFiles", "MsiInput", msiFileName);
         }