From 6e8c6a15ad01cd9dde6e0ea08f93132c7778658d Mon Sep 17 00:00:00 2001
From: Chris Bednarski <banzaimonkey@gmail.com>
Date: Fri, 14 Aug 2015 17:49:08 -0700
Subject: [PATCH] Implement fix, add comments so it's more apparent why we're
 doing special logic

---
 common/download.go      | 11 +++++++++--
 common/download_test.go | 16 ++++++++--------
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/common/download.go b/common/download.go
index 184624ffe..5f213968b 100644
--- a/common/download.go
+++ b/common/download.go
@@ -115,7 +115,10 @@ func (d *DownloadClient) Get() (string, error) {
 	// Files when we don't copy the file are special cased.
 	var f *os.File
 	var finalPath string
+	sourcePath := ""
 	if url.Scheme == "file" && !d.config.CopyFile {
+		// This is a special case where we use a source file that already exists
+		// locally and we don't make a copy. Normally we would copy or download.
 		finalPath = url.Path
 		log.Printf("Using local file: %s", finalPath)
 
@@ -123,6 +126,8 @@ func (d *DownloadClient) Get() (string, error) {
 		if runtime.GOOS == "windows" && len(finalPath) > 0 && finalPath[0] == '/' {
 			finalPath = finalPath[1:len(finalPath)]
 		}
+		// Keep track of the source so we can make sure not to delete this later
+		sourcePath = finalPath
 	} else {
 		finalPath = d.config.TargetPath
 
@@ -150,8 +155,10 @@ func (d *DownloadClient) Get() (string, error) {
 		var verify bool
 		verify, err = d.VerifyChecksum(finalPath)
 		if err == nil && !verify {
-			// Delete the file
-			os.Remove(finalPath)
+			// Only delete the file if we made a copy or downloaded it
+			if sourcePath != finalPath {
+				os.Remove(finalPath)
+			}
 
 			err = fmt.Errorf(
 				"checksums didn't match expected: %s",
diff --git a/common/download_test.go b/common/download_test.go
index f77956913..51f6f270c 100644
--- a/common/download_test.go
+++ b/common/download_test.go
@@ -340,6 +340,10 @@ func TestHashForType(t *testing.T) {
 	}
 }
 
+// TestDownloadFileUrl tests a special case where we use a local file for
+// iso_url. In this case we can still verify the checksum but we should not
+// delete the file if the checksum fails. Instead we'll just error and let the
+// user fix the checksum.
 func TestDownloadFileUrl(t *testing.T) {
 	cwd, err := os.Getwd()
 	if err != nil {
@@ -361,14 +365,10 @@ func TestDownloadFileUrl(t *testing.T) {
 
 	client := NewDownloadClient(config)
 
-	filename, err := client.Get()
-	defer os.Remove(config.TargetPath)
-	if err != nil {
-		t.Fatalf("Failed to download test file")
-	}
-
-	if sourcePath != filename {
-		t.Errorf("Filename doesn't match; expected %s got %s", sourcePath, filename)
+	// Verify that we fail to match the checksum
+	_, err = client.Get()
+	if err.Error() != "checksums didn't match expected: 6e6f7065" {
+		t.Fatalf("Unexpected failure; expected checksum not to match")
 	}
 
 	if _, err = os.Stat(sourcePath); err != nil {