Merge branch 'hashicorpGH-2377' into release-0.9.0

builder/vmware/iso/step_create_vmx.go

@@ -43,6 +43,11 @@ func (s *stepCreateVMX) Run(state multistep.StateBag) multistep.StepAction {
 	isoPath := state.Get("iso_path").(string)
 	ui := state.Get("ui").(packer.Ui)
 
+	// Convert the iso_path into a path relative to the .vmx file if possible
+	if relativeIsoPath,err := filepath.Rel(config.VMXTemplatePath, filepath.FromSlash(isoPath)); err == nil {
+		isoPath = relativeIsoPath
+	}
+
 	ui.Say("Building and writing VMX file")
 
 	vmxTemplate := DefaultVMXTemplate

common/config.go

@@ -42,84 +42,110 @@ func DownloadableURL(original string) (string, error) {
 		// we're dealing with a drive letter and thus a file path.
 		idx := strings.Index(original, ":")
 		if idx == 1 {
-			original = "file:///" + original
+			original = "file://" + filepath.ToSlash(original)
 		}
 	}
 
-	url, err := url.Parse(original)
+	// XXX: The validation here is later re-parsed in common/download.go and
+	//      thus any modifications here must remain consistent over there too.
+	uri, err := url.Parse(original)
 	if err != nil {
 		return "", err
 	}
 
-	if url.Scheme == "" {
-		url.Scheme = "file"
+	if uri.Scheme == "" {
+		uri.Scheme = "file"
 	}
 
-	if url.Scheme == "file" {
-		// Windows file handling is all sorts of tricky...
+	const UNCPrefix = string(os.PathSeparator)+string(os.PathSeparator)
+	if uri.Scheme == "file" {
+		var ospath string	// os-formatted pathname
 		if runtime.GOOS == "windows" {
-			// If the path is using Windows-style slashes, URL parses
-			// it into the host field.
-			if url.Path == "" && strings.Contains(url.Host, `\`) {
-				url.Path = url.Host
-				url.Host = ""
-			}
-
-			// For Windows absolute file paths, remove leading / prior to processing
-			// since net/url turns "C:/" into "/C:/"
-			if len(url.Path) > 0 && url.Path[0] == '/' {
-				url.Path = url.Path[1:len(url.Path)]
+			// Move any extra path components that were mis-parsed into the Host
+			// field back into the uri.Path field
+			if len(uri.Host) >= len(UNCPrefix) && uri.Host[:len(UNCPrefix)] == UNCPrefix {
+				idx := strings.Index(uri.Host[len(UNCPrefix):], string(os.PathSeparator))
+				if idx > -1 {
+					uri.Path = filepath.ToSlash(uri.Host[idx+len(UNCPrefix):]) + uri.Path
+					uri.Host = uri.Host[:idx+len(UNCPrefix)]
+				}
 			}
+			// Now all we need to do to convert the uri to a platform-specific path
+			// is to trade it's slashes for some os.PathSeparator ones.
+			ospath = uri.Host + filepath.FromSlash(uri.Path)
+
+		} else {
+			// Since we're already using sane paths on a sane platform, anything in
+			// uri.Host can be assumed that the user is describing a relative uri.
+			// This means that if we concatenate it with uri.Path, the filepath
+			// transform will still open the file correctly.
+			//     i.e. file://localdirectory/filename -> localdirectory/filename
+			ospath = uri.Host + uri.Path
 		}
 
 		// Only do the filepath transformations if the file appears
-		// to actually exist.
-		if _, err := os.Stat(url.Path); err == nil {
-			url.Path, err = filepath.Abs(url.Path)
+		// to actually exist. We don't do it on windows, because EvalSymlinks
+		// won't understand how to handle UNC paths and other Windows-specific minutae.
+		if _, err := os.Stat(ospath); err == nil && runtime.GOOS != "windows" {
+			ospath, err = filepath.Abs(ospath)
 			if err != nil {
 				return "", err
 			}
 
-			url.Path, err = filepath.EvalSymlinks(url.Path)
+			ospath, err = filepath.EvalSymlinks(ospath)
 			if err != nil {
 				return "", err
 			}
 
-			url.Path = filepath.Clean(url.Path)
+			ospath = filepath.Clean(ospath)
 		}
 
+		// now that ospath was normalized and such..
 		if runtime.GOOS == "windows" {
-			// Also replace all backslashes with forwardslashes since Windows
-			// users are likely to do this but the URL should actually only
-			// contain forward slashes.
-			url.Path = strings.Replace(url.Path, `\`, `/`, -1)
+			uri.Host = ""
+			// Check to see if our ospath is unc-prefixed, and if it is then split
+			// the UNC host into uri.Host, leaving the rest in ospath.
+			// This way, our UNC-uri is protected from injury in the call to uri.String()
+			if len(ospath) >= len(UNCPrefix) && ospath[:len(UNCPrefix)] == UNCPrefix {
+				idx := strings.Index(ospath[len(UNCPrefix):], string(os.PathSeparator))
+				if idx > -1 {
+					uri.Host = ospath[:len(UNCPrefix)+idx]
+					ospath = ospath[len(UNCPrefix)+idx:]
+				}
+			}
+			// Restore the uri by re-transforming our os-formatted path
+			uri.Path = filepath.ToSlash(ospath)
+		} else {
+			uri.Host = ""
+			uri.Path = filepath.ToSlash(ospath)
 		}
 	}
 
 	// Make sure it is lowercased
-	url.Scheme = strings.ToLower(url.Scheme)
-
-	// This is to work around issue #5927. This can safely be removed once
-	// we distribute with a version of Go that fixes that bug.
-	//
-	// See: https://code.google.com/p/go/issues/detail?id=5927
-	if url.Path != "" && url.Path[0] != '/' {
-		url.Path = "/" + url.Path
-	}
+	uri.Scheme = strings.ToLower(uri.Scheme)
 
 	// Verify that the scheme is something we support in our common downloader.
 	supported := []string{"file", "http", "https"}
 	found := false
 	for _, s := range supported {
-		if url.Scheme == s {
+		if uri.Scheme == s {
 			found = true
 			break
 		}
 	}
 
 	if !found {
-		return "", fmt.Errorf("Unsupported URL scheme: %s", url.Scheme)
+		return "", fmt.Errorf("Unsupported URL scheme: %s", uri.Scheme)
+	}
+
+	// explicit check to see if we need to manually replace the uri host with a UNC one
+	if runtime.GOOS == "windows" && uri.Scheme == "file" {
+		if len(uri.Host) >= len(UNCPrefix) && uri.Host[:len(UNCPrefix)] == UNCPrefix {
+			escapedHost := url.QueryEscape(uri.Host)
+			return strings.Replace(uri.String(), escapedHost, uri.Host, 1), nil
+		}
 	}
 
-	return url.String(), nil
+	// otherwise, we can trust the url handler
+	return uri.String(), nil
 }

common/download.go

@@ -16,6 +16,9 @@ import (
 	"net/url"
 	"os"
 	"runtime"
+	"path"
+	"path/filepath"
+	"strings"
 )
 
 // DownloadConfig is the configuration given to instantiate a new
@@ -98,6 +101,44 @@ func (d *DownloadClient) Cancel() {
 	// TODO(mitchellh): Implement
 }
 
+// Take a uri and convert it to a path that makes sense on the Windows platform
+func NormalizeWindowsURL(basepath string, url url.URL) string {
+	// This logic must correspond to the same logic in the NormalizeWindowsURL
+	// function found in common/config.go since that function _also_ checks that
+	// the url actually exists in file form.
+
+	const UNCPrefix = string(os.PathSeparator)+string(os.PathSeparator)
+
+	// move any extra path components that were parsed into Host due
+	// to UNC into the url.Path field so that it's PathSeparators get
+	// normalized
+	if len(url.Host) >= len(UNCPrefix) && url.Host[:len(UNCPrefix)] == UNCPrefix {
+		idx := strings.Index(url.Host[len(UNCPrefix):], string(os.PathSeparator))
+		if idx > -1 {
+			url.Path = filepath.ToSlash(url.Host[idx+len(UNCPrefix):]) + url.Path
+			url.Host = url.Host[:idx+len(UNCPrefix)]
+		}
+	}
+
+	// clean up backward-slashes since they only matter when part of a unc path
+	urlPath := filepath.ToSlash(url.Path)
+
+	// semi-absolute path (current drive letter) -- file:///absolute/path
+	if url.Host == "" && len(urlPath) > 0 && urlPath[0] == '/' {
+		return path.Join(filepath.VolumeName(basepath), urlPath)
+
+	// relative path -- file://./relative/path
+	//					file://relative/path
+	} else if url.Host == "" || (len(url.Host) > 0 && url.Host[0] == '.') {
+		return path.Join(filepath.ToSlash(basepath), urlPath)
+	}
+
+	// absolute path
+		// UNC -- file://\\host/share/whatever
+		// drive -- file://c:/absolute/path
+	return path.Join(url.Host, urlPath)
+}
+
 func (d *DownloadClient) Get() (string, error) {
 	// If we already have the file and it matches, then just return the target path.
 	if verify, _ := d.VerifyChecksum(d.config.TargetPath); verify {
@@ -122,10 +163,19 @@ func (d *DownloadClient) Get() (string, error) {
 		finalPath = url.Path
 		log.Printf("[DEBUG] Using local file: %s", finalPath)
 
-		// Remove forward slash on absolute Windows file URLs before processing
-		if runtime.GOOS == "windows" && len(finalPath) > 0 && finalPath[0] == '/' {
-			finalPath = finalPath[1:len(finalPath)]
+		// transform the actual file uri to a windowsy path if we're being windowsy.
+		if runtime.GOOS == "windows" {
+			// FIXME: cwd should point to a path relative to the TEMPLATE path,
+			//        but since this isn't exposed to us anywhere, we use os.Getwd()
+			//        and assume the user ran packer in the same directory that
+			//        any relative files are located at.
+			cwd,err := os.Getwd()
+			if err != nil {
+				return "", fmt.Errorf("Unable to get working directory")
+			}
+			finalPath = NormalizeWindowsURL(cwd, *url)
 		}
+
 		// Keep track of the source so we can make sure not to delete this later
 		sourcePath = finalPath
 	} else {

common/download_test.go

@@ -8,6 +8,8 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"os"
+	"strings"
+	"path/filepath"
 	"testing"
 )
 
@@ -374,5 +376,106 @@ func TestDownloadFileUrl(t *testing.T) {
 	if _, err = os.Stat(sourcePath); err != nil {
 		t.Errorf("Could not stat source file: %s", sourcePath)
 	}
+}
 
+// SimulateFileUriDownload is a simple utility function that converts a uri
+// into a testable file path whilst ignoring a correct checksum match, stripping
+// UNC path info, and then calling stat to ensure the correct file exists.
+//    (used by TestFileUriTransforms)
+func SimulateFileUriDownload(t *testing.T, uri string) (string,error) {
+	// source_path is a file path and source is a network path
+	source := fmt.Sprintf(uri)
+	t.Logf("Trying to download %s", source)
+
+	config := &DownloadConfig{
+		Url: source,
+		// This should be wrong. We want to make sure we don't delete
+		Checksum: []byte("nope"),
+		Hash:     HashForType("sha256"),
+		CopyFile: false,
+	}
+
+	// go go go
+	client := NewDownloadClient(config)
+	path, err := client.Get()
+
+	// ignore any non-important checksum errors if it's not a unc path
+	if !strings.HasPrefix(path, "\\\\") && err.Error() != "checksums didn't match expected: 6e6f7065" {
+		t.Fatalf("Unexpected failure; expected checksum not to match")
+	}
+
+	// if it's a unc path, then remove the host and share name so we don't have
+	// to force the user to enable ADMIN$ and Windows File Sharing
+	if strings.HasPrefix(path, "\\\\") {
+		res := strings.SplitN(path, "/", 3)
+		path = "/" + res[2]
+	}
+
+	if _, err = os.Stat(path); err != nil {
+		t.Errorf("Could not stat source file: %s", path)
+	}
+	return path,err
+}
+
+// TestFileUriTransforms tests the case where we use a local file uri
+// for iso_url. There's a few different formats that a file uri can exist as
+// and so we try to test the most useful and common ones.
+func TestFileUriTransforms(t *testing.T) {
+	const testpath = /* have your */ "test-fixtures/fileurl/cake" /* and eat it too */
+	const host = "localhost"
+
+	var cwd string
+	var volume string
+	var share string
+
+	cwd, err := os.Getwd()
+	if err != nil {
+		t.Fatalf("Unable to detect working directory: %s", err)
+		return
+	}
+	cwd = filepath.ToSlash(cwd)
+	volume = filepath.VolumeName(cwd)
+	share = volume
+	if share[len(share)-1] == ':' {
+		share = share[:len(share)-1] + "$"
+	}
+	cwd = cwd[len(volume):]
+
+	t.Logf("TestFileUriTransforms : Running with cwd : '%s'", cwd)
+	t.Logf("TestFileUriTransforms : Running with volume : '%s'", volume)
+
+	// ./relative/path -> ./relative/path
+	// /absolute/path -> /absolute/path
+	// c:/windows/absolute -> c:/windows/absolute
+	// \\host/sharename/file -> \\host/sharename/file
+	testcases := []string{
+		"./%s",
+		cwd + "/%s",
+		volume + cwd + "/%s",
+		"\\\\" + host + "/" + share + "/" + cwd[1:] + "/%s",
+	}
+
+	// all regular slashed testcases
+	for _,testcase := range testcases {
+		uri := "file://" + fmt.Sprintf(testcase, testpath)
+		t.Logf("TestFileUriTransforms : Trying Uri '%s'", uri)
+		res,err := SimulateFileUriDownload(t, uri)
+		if err != nil {
+			t.Errorf("Unable to transform uri '%s' into a path : %v", uri, err)
+		}
+		t.Errorf("TestFileUriTransforms : Result Path '%s'", res)
+	}
+
+	// ...and finally the oddball windows native path
+	// \\host\sharename\file -> \\host/sharename/file
+	testpath_native := filepath.FromSlash(testpath)
+	testcase_native := "\\\\" + host + "\\" + share + "\\" + filepath.FromSlash(cwd[1:]) + "\\%s"
+	uri := "file://" + fmt.Sprintf(testcase_native, testpath_native)
+	t.Logf("TestFileUriTransforms : Trying Uri '%s'", uri)
+	res,err := SimulateFileUriDownload(t, uri)
+	if err != nil {
+		t.Errorf("Unable to transform uri '%s' into a path", uri)
+		return
+	}
+	t.Errorf("TestFileUriTransforms : Result Path '%s'", res)
 }

packer/core.go

@@ -67,7 +67,7 @@ func NewCore(c *CoreConfig) (*Core, error) {
 		return nil, err
 	}
 
-	// Go through and interpolate all the build names. We shuld be able
+	// Go through and interpolate all the build names. We should be able
 	// to do this at this point with the variables.
 	result.builds = make(map[string]*template.Builder)
 	for _, b := range c.Template.Builders {