You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by GitBox <gi...@apache.org> on 2017/11/28 02:19:53 UTC

[GitHub] ccollins476ad closed pull request #109: newt - Delete temporary directory after use

ccollins476ad closed pull request #109: newt - Delete temporary directory after use
URL: https://github.com/apache/mynewt-newt/pull/109
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/newt/cli/project_cmds.go b/newt/cli/project_cmds.go
index 7716556..e900985 100644
--- a/newt/cli/project_cmds.go
+++ b/newt/cli/project_cmds.go
@@ -51,15 +51,20 @@ func newRunCmd(cmd *cobra.Command, args []string) {
 	dl.User = "apache"
 	dl.Repo = "mynewt-blinky"
 
-	dir, err := dl.DownloadRepo(newtutil.NewtBlinkyTag)
+	tmpdir, err := newtutil.MakeTempRepoDir()
 	if err != nil {
-		NewtUsage(cmd, err)
+		NewtUsage(nil, err)
+	}
+	defer os.RemoveAll(tmpdir)
+
+	if err := dl.DownloadRepo(newtutil.NewtBlinkyTag, tmpdir); err != nil {
+		NewtUsage(nil, err)
 	}
 
 	util.StatusMessage(util.VERBOSITY_DEFAULT, "Installing "+
 		"skeleton in %s...\n", newDir)
 
-	if err := util.CopyDir(dir, newDir); err != nil {
+	if err := util.CopyDir(tmpdir, newDir); err != nil {
 		NewtUsage(cmd, err)
 	}
 
diff --git a/newt/downloader/downloader.go b/newt/downloader/downloader.go
index 471dfb8..8475fdf 100644
--- a/newt/downloader/downloader.go
+++ b/newt/downloader/downloader.go
@@ -37,7 +37,7 @@ type Downloader interface {
 	FetchFile(path string, filename string, dstDir string) error
 	Branch() string
 	SetBranch(branch string)
-	DownloadRepo(branch string) (string, error)
+	DownloadRepo(commit string, dstPath string) error
 	CurrentBranch(path string) (string, error)
 	UpdateRepo(path string, branchName string) error
 	CleanupRepo(path string, branchName string) error
@@ -433,16 +433,10 @@ func (gd *GithubDownloader) setRemoteAuth(path string) error {
 	return gd.setOriginUrl(path, url)
 }
 
-func (gd *GithubDownloader) DownloadRepo(commit string) (string, error) {
+func (gd *GithubDownloader) DownloadRepo(commit string, dstPath string) error {
 	// Currently only the master branch is supported.
 	branch := "master"
 
-	// Get a temporary directory, and copy the repository into that directory.
-	tmpdir, err := ioutil.TempDir("", "newt-repo")
-	if err != nil {
-		return "", err
-	}
-
 	url, publicUrl := gd.remoteUrls()
 
 	util.StatusMessage(util.VERBOSITY_VERBOSE, "Downloading "+
@@ -451,7 +445,7 @@ func (gd *GithubDownloader) DownloadRepo(commit string) (string, error) {
 
 	gp, err := gitPath()
 	if err != nil {
-		return "", err
+		return err
 	}
 
 	// Clone the repository.
@@ -461,7 +455,7 @@ func (gd *GithubDownloader) DownloadRepo(commit string) (string, error) {
 		"-b",
 		branch,
 		url,
-		tmpdir,
+		dstPath,
 	}
 
 	if util.Verbosity >= util.VERBOSITY_VERBOSE {
@@ -470,19 +464,17 @@ func (gd *GithubDownloader) DownloadRepo(commit string) (string, error) {
 		_, err = util.ShellCommand(cmd, nil)
 	}
 	if err != nil {
-		os.RemoveAll(tmpdir)
-		return "", err
+		return err
 	}
 
-	defer gd.clearRemoteAuth(tmpdir)
+	defer gd.clearRemoteAuth(dstPath)
 
 	// Checkout the specified commit.
-	if err := checkout(tmpdir, commit); err != nil {
-		os.RemoveAll(tmpdir)
-		return "", err
+	if err := checkout(dstPath, commit); err != nil {
+		return err
 	}
 
-	return tmpdir, nil
+	return nil
 }
 
 func NewGithubDownloader() *GithubDownloader {
@@ -569,22 +561,16 @@ func (gd *GitDownloader) AreChanges(path string) (bool, error) {
 	return areChanges(path)
 }
 
-func (gd *GitDownloader) DownloadRepo(commit string) (string, error) {
+func (gd *GitDownloader) DownloadRepo(commit string, dstPath string) error {
 	// Currently only the master branch is supported.
 	branch := "master"
 
-	// Get a temporary directory, and copy the repository into that directory.
-	tmpdir, err := ioutil.TempDir("", "newt-repo")
-	if err != nil {
-		return "", err
-	}
-
 	util.StatusMessage(util.VERBOSITY_VERBOSE, "Downloading "+
 		"repository %s (branch: %s; commit: %s)\n", gd.Url, branch, commit)
 
 	gp, err := gitPath()
 	if err != nil {
-		return "", err
+		return err
 	}
 
 	// Clone the repository.
@@ -594,7 +580,7 @@ func (gd *GitDownloader) DownloadRepo(commit string) (string, error) {
 		"-b",
 		branch,
 		gd.Url,
-		tmpdir,
+		dstPath,
 	}
 
 	if util.Verbosity >= util.VERBOSITY_VERBOSE {
@@ -603,17 +589,15 @@ func (gd *GitDownloader) DownloadRepo(commit string) (string, error) {
 		_, err = util.ShellCommand(cmd, nil)
 	}
 	if err != nil {
-		os.RemoveAll(tmpdir)
-		return "", err
+		return err
 	}
 
 	// Checkout the specified commit.
-	if err := checkout(tmpdir, commit); err != nil {
-		os.RemoveAll(tmpdir)
-		return "", err
+	if err := checkout(dstPath, commit); err != nil {
+		return err
 	}
 
-	return tmpdir, nil
+	return nil
 }
 
 func NewGitDownloader() *GitDownloader {
@@ -647,8 +631,14 @@ func (ld *LocalDownloader) UpdateRepo(path string, branchName string) error {
 
 func (ld *LocalDownloader) CleanupRepo(path string, branchName string) error {
 	os.RemoveAll(path)
-	_, err := ld.DownloadRepo(branchName)
-	return err
+
+	tmpdir, err := newtutil.MakeTempRepoDir()
+	if err != nil {
+		return err
+	}
+	defer os.RemoveAll(tmpdir)
+
+	return ld.DownloadRepo(branchName, tmpdir)
 }
 
 func (ld *LocalDownloader) LocalDiff(path string) ([]byte, error) {
@@ -659,26 +649,20 @@ func (ld *LocalDownloader) AreChanges(path string) (bool, error) {
 	return areChanges(path)
 }
 
-func (ld *LocalDownloader) DownloadRepo(commit string) (string, error) {
-	// Get a temporary directory, and copy the repository into that directory.
-	tmpdir, err := ioutil.TempDir("", "newt-repo")
-	if err != nil {
-		return "", err
-	}
-
+func (ld *LocalDownloader) DownloadRepo(commit string, dstPath string) error {
 	util.StatusMessage(util.VERBOSITY_VERBOSE,
 		"Downloading local repository %s\n", ld.Path)
 
-	if err := util.CopyDir(ld.Path, tmpdir); err != nil {
-		return "", err
+	if err := util.CopyDir(ld.Path, dstPath); err != nil {
+		return err
 	}
 
 	// Checkout the specified commit.
-	if err := checkout(tmpdir, commit); err != nil {
-		return "", err
+	if err := checkout(dstPath, commit); err != nil {
+		return err
 	}
 
-	return tmpdir, nil
+	return nil
 }
 
 func NewLocalDownloader() *LocalDownloader {
diff --git a/newt/newtutil/newtutil.go b/newt/newtutil/newtutil.go
index 4d9fef1..b092a8b 100644
--- a/newt/newtutil/newtutil.go
+++ b/newt/newtutil/newtutil.go
@@ -21,6 +21,7 @@ package newtutil
 
 import (
 	"fmt"
+	"io/ioutil"
 	"os/user"
 	"sort"
 	"strconv"
@@ -306,3 +307,13 @@ func GeneratedPreamble() string {
 	return fmt.Sprintf("/**\n * This file was generated by %s\n */\n\n",
 		NewtVersionStr)
 }
+
+// Creates a temporary directory for downloading a repo.
+func MakeTempRepoDir() (string, error) {
+	tmpdir, err := ioutil.TempDir("", "newt-repo")
+	if err != nil {
+		return "", util.ChildNewtError(err)
+	}
+
+	return tmpdir, nil
+}
diff --git a/newt/project/pkgwriter.go b/newt/project/pkgwriter.go
index d912d6f..ba10d39 100644
--- a/newt/project/pkgwriter.go
+++ b/newt/project/pkgwriter.go
@@ -28,6 +28,7 @@ import (
 	"strings"
 
 	"mynewt.apache.org/newt/newt/downloader"
+	"mynewt.apache.org/newt/newt/newtutil"
 	"mynewt.apache.org/newt/util"
 )
 
@@ -242,10 +243,15 @@ func (pw *PackageWriter) WritePackage() error {
 		"Download package template for package type %s.\n",
 		strings.ToLower(pw.template))
 
-	tmpdir, err := dl.DownloadRepo(pw.repo.branch)
+	tmpdir, err := newtutil.MakeTempRepoDir()
 	if err != nil {
 		return err
 	}
+	defer os.RemoveAll(tmpdir)
+
+	if err := dl.DownloadRepo(pw.repo.branch, tmpdir); err != nil {
+		return err
+	}
 
 	if err := os.RemoveAll(tmpdir + "/.git/"); err != nil {
 		return util.NewNewtError(err.Error())
diff --git a/newt/repo/repo.go b/newt/repo/repo.go
index 3e31b15..bea592e 100644
--- a/newt/repo/repo.go
+++ b/newt/repo/repo.go
@@ -433,10 +433,15 @@ func (r *Repo) patchesFilePath() string {
 func (r *Repo) downloadRepo(branchName string) error {
 	dl := r.downloader
 
-	// Download the git repo, returns the git repo, checked out to that branch
-	tmpdir, err := dl.DownloadRepo(branchName)
+	tmpdir, err := newtutil.MakeTempRepoDir()
 	if err != nil {
-		return util.FmtNewtError("Error download repository %s, : %s",
+		return err
+	}
+	defer os.RemoveAll(tmpdir)
+
+	// Download the git repo, returns the git repo, checked out to that branch
+	if err := dl.DownloadRepo(branchName, tmpdir); err != nil {
+		return util.FmtNewtError("Error downloading repository %s, : %s",
 			r.Name(), err.Error())
 	}
 


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services