Repository: incubator-mynewt-newt
Updated Branches:
  refs/heads/develop 037457dd6 -> 0241d2f38


MYNEWT-365 newt can't handle files with spaces

Newt no longer invokes /bin/sh to execute external commands.  Instead,
it now uses the Go exec library to fork+exec child processes.

NOTE: This commit breaks backwards-compatibility with the Mynewt core
repo.  Specifically, the compiler.yml definitions indicated command line
options as a space-delimited sequence of strings in the path settings.
E.g., the Cortex M4 assembler specified the following path:

    compiler.path.as: arm-none-eabi-gcc -x assembler-with-cpp

This compiler definition has now been changed to the following:

    compiler.path.as: arm-none-eabi-gcc
    compiler.as.flags: [-x, assembler-with-cpp]

We could have maintained backwards-compatibility by splitting paths on
spaces and interpreting secondary tokens as command-line options.
However, this approach prevents us from specifying a compiler path
that actually contains spaces.  I think paths with spaces is not
uncommon (e.g., "/Applications/GNU ARM Eclipse/...")

In addition, some packages' cflags contained shell-escaped strings
(e.g., backslash-quote).  These escapes had to be removed now that the
flags aren't passing through the shell.


Project: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-newt/repo
Commit: 
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-newt/commit/0241d2f3
Tree: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-newt/tree/0241d2f3
Diff: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-newt/diff/0241d2f3

Branch: refs/heads/develop
Commit: 0241d2f38e8236f03d6f20d5c7198d4a0a4cb169
Parents: 037457d
Author: Christopher Collins <ccoll...@apache.org>
Authored: Thu Dec 1 11:00:40 2016 -0800
Committer: Christopher Collins <ccoll...@apache.org>
Committed: Thu Dec 1 11:50:03 2016 -0800

----------------------------------------------------------------------
 newt/builder/build.go                           |   3 +-
 newt/builder/load.go                            |  25 +-
 newt/downloader/downloader.go                   |  11 +-
 newt/image/image.go                             |  37 ++-
 newt/toolchain/compiler.go                      | 265 +++++++++++++------
 newt/toolchain/deps.go                          |  13 +-
 newt/vendor/mynewt.apache.org/newt/util/util.go |  62 ++++-
 util/util.go                                    |  62 ++++-
 8 files changed, 358 insertions(+), 120 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-mynewt-newt/blob/0241d2f3/newt/builder/build.go
----------------------------------------------------------------------
diff --git a/newt/builder/build.go b/newt/builder/build.go
index 82798fb..230d988 100644
--- a/newt/builder/build.go
+++ b/newt/builder/build.go
@@ -547,7 +547,8 @@ func (b *Builder) Test(p *pkg.LocalPackage) error {
 
        util.StatusMessage(util.VERBOSITY_DEFAULT, "Executing test: %s\n",
                testFilename)
-       if _, err := util.ShellCommand(testFilename); err != nil {
+       cmd := []string{testFilename}
+       if _, err := util.ShellCommand(cmd, nil); err != nil {
                newtError := err.(*util.NewtError)
                newtError.Text = fmt.Sprintf("Test failure (%s):\n%s", p.Name(),
                        newtError.Text)

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-newt/blob/0241d2f3/newt/builder/load.go
----------------------------------------------------------------------
diff --git a/newt/builder/load.go b/newt/builder/load.go
index ec07f1f..b7ae34e 100644
--- a/newt/builder/load.go
+++ b/newt/builder/load.go
@@ -24,6 +24,7 @@ import (
        "os"
        "sort"
        "strconv"
+       "strings"
 
        "mynewt.apache.org/newt/newt/pkg"
        "mynewt.apache.org/newt/newt/project"
@@ -65,25 +66,27 @@ func Load(binBaseName string, bspPkg *pkg.BspPackage,
        }
        sort.Strings(sortedKeys)
 
-       envSettings := ""
+       env := []string{}
        for _, key := range sortedKeys {
-               envSettings += fmt.Sprintf("%s=\"%s\" ", key, 
extraEnvSettings[key])
+               env = append(env, fmt.Sprintf("%s=%s", key, 
extraEnvSettings[key]))
        }
 
        coreRepo := project.GetProject().FindRepo("apache-mynewt-core")
-       envSettings += fmt.Sprintf("CORE_PATH=\"%s\" ", coreRepo.Path())
-       envSettings += fmt.Sprintf("BSP_PATH=\"%s\" ", bspPath)
-       envSettings += fmt.Sprintf("BIN_BASENAME=\"%s\" ", binBaseName)
+       env = append(env, fmt.Sprintf("CORE_PATH=%s", coreRepo.Path()))
+       env = append(env, fmt.Sprintf("BSP_PATH=%s", bspPath))
+       env = append(env, fmt.Sprintf("BIN_BASENAME=%s", binBaseName))
 
        // bspPath, binBaseName are passed in command line for backwards
        // compatibility
-       downloadCmd := fmt.Sprintf("%s %s %s %s", envSettings,
-               bspPkg.DownloadScript, bspPath, binBaseName)
+       cmd := []string{
+               bspPkg.DownloadScript,
+               bspPath,
+               binBaseName,
+       }
 
        util.StatusMessage(util.VERBOSITY_VERBOSE, "Load command: %s\n",
-               downloadCmd)
-       _, err := util.ShellCommand(downloadCmd)
-       if err != nil {
+               strings.Join(cmd, " "))
+       if _, err := util.ShellCommand(cmd, env); err != nil {
                return err
        }
        util.StatusMessage(util.VERBOSITY_VERBOSE, "Successfully loaded 
image.\n")
@@ -183,7 +186,7 @@ func (b *Builder) Debug(extraJtagCmd string, reset bool, 
noGDB bool) error {
                fmt.Sprintf("CORE_PATH=%s", coreRepo.Path()),
                fmt.Sprintf("BSP_PATH=%s", bspPath),
                fmt.Sprintf("BIN_BASENAME=%s", binBaseName),
-               fmt.Sprintf("FEATURES=\"%s\"", featureString),
+               fmt.Sprintf("FEATURES=%s", featureString),
        }
        if extraJtagCmd != "" {
                envSettings = append(envSettings,

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-newt/blob/0241d2f3/newt/downloader/downloader.go
----------------------------------------------------------------------
diff --git a/newt/downloader/downloader.go b/newt/downloader/downloader.go
index 7849feb..7b073bb 100644
--- a/newt/downloader/downloader.go
+++ b/newt/downloader/downloader.go
@@ -26,7 +26,6 @@ import (
        "net/http"
        "os"
        "os/exec"
-       "strings"
 
        log "github.com/Sirupsen/logrus"
 
@@ -84,13 +83,13 @@ func checkout(repoDir string, commit string) error {
        }
 
        // Checkout the specified commit.
-       cmds := []string{
+       cmd := []string{
                gitPath,
                "checkout",
                commit,
        }
 
-       if o, err := util.ShellCommand(strings.Join(cmds, " ")); err != nil {
+       if o, err := util.ShellCommand(cmd, nil); err != nil {
                return util.NewNewtError(string(o))
        }
 
@@ -187,7 +186,7 @@ func (gd *GithubDownloader) DownloadRepo(commit string) 
(string, error) {
        }
 
        // Clone the repository.
-       cmds := []string{
+       cmd := []string{
                gitPath,
                "clone",
                "-b",
@@ -197,12 +196,12 @@ func (gd *GithubDownloader) DownloadRepo(commit string) 
(string, error) {
        }
 
        if util.Verbosity >= util.VERBOSITY_VERBOSE {
-               if err := util.ShellInteractiveCommand(cmds, nil); err != nil {
+               if err := util.ShellInteractiveCommand(cmd, nil); err != nil {
                        os.RemoveAll(tmpdir)
                        return "", err
                }
        } else {
-               if _, err := util.ShellCommand(strings.Join(cmds, " ")); err != 
nil {
+               if _, err := util.ShellCommand(cmd, nil); err != nil {
                        return "", err
                }
        }

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-newt/blob/0241d2f3/newt/image/image.go
----------------------------------------------------------------------
diff --git a/newt/image/image.go b/newt/image/image.go
index 1e5e100..24e37c8 100644
--- a/newt/image/image.go
+++ b/newt/image/image.go
@@ -481,15 +481,36 @@ func (r *RepoManager) GetImageManifestPkg(
                Name: ip.Repo,
        }
 
-       res, err := util.ShellCommand(fmt.Sprintf("cd %s && git rev-parse HEAD",
-               path))
+       // Make sure we restore the current working dir to whatever it was when
+       // this function was called
+       cwd, err := os.Getwd()
+       if err != nil {
+               log.Debugf("Unable to determine current working directory: %v", 
err)
+               return ip
+       }
+       defer os.Chdir(cwd)
+
+       if err := os.Chdir(path); err != nil {
+               return ip
+       }
+
+       var res []byte
+
+       res, err = util.ShellCommand([]string{
+               "git",
+               "rev-parse",
+               "HEAD",
+       }, nil)
        if err != nil {
                log.Debugf("Unable to determine commit hash for %s: %v", path, 
err)
                repo.Commit = "UNKNOWN"
        } else {
                repo.Commit = strings.TrimSpace(string(res))
-               res, err := util.ShellCommand(fmt.Sprintf(
-                       "cd %s && git status --porcelain", path))
+               res, err = util.ShellCommand([]string{
+                       "git",
+                       "status",
+                       "--porcelain",
+               }, nil)
                if err != nil {
                        log.Debugf("Unable to determine dirty state for %s: 
%v", path, err)
                } else {
@@ -497,8 +518,12 @@ func (r *RepoManager) GetImageManifestPkg(
                                repo.Dirty = true
                        }
                }
-               res, err = util.ShellCommand(fmt.Sprintf(
-                       "cd %s && git config --get remote.origin.url", path))
+               res, err = util.ShellCommand([]string{
+                       "git",
+                       "config",
+                       "--get",
+                       "remote.origin.url",
+               }, nil)
                if err != nil {
                        log.Debugf("Unable to determine URL for %s: %v", path, 
err)
                } else {

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-newt/blob/0241d2f3/newt/toolchain/compiler.go
----------------------------------------------------------------------
diff --git a/newt/toolchain/compiler.go b/newt/toolchain/compiler.go
index 42af123..ed34254 100644
--- a/newt/toolchain/compiler.go
+++ b/newt/toolchain/compiler.go
@@ -306,16 +306,48 @@ func (c *Compiler) includesString() string {
        return "-I" + strings.Join(includes, " -I")
 }
 
+// Generates a string consisting of all the necessary include path (-I)
+// options.  The result is sorted and contains no duplicate paths.
+func (c *Compiler) includesStrings() []string {
+       if len(c.info.Includes) == 0 {
+               return nil
+       }
+
+       includes := util.SortFields(c.info.Includes...)
+
+       tokens := make([]string, len(includes))
+       for i, s := range includes {
+               tokens[i] = "-I" + s
+       }
+
+       return tokens
+}
+
 func (c *Compiler) cflagsString() string {
        cflags := util.SortFields(c.info.Cflags...)
        return strings.Join(cflags, " ")
 }
 
+func (c *Compiler) cflagsStrings() []string {
+       cflags := util.SortFields(c.info.Cflags...)
+       return cflags
+}
+
+func (c *Compiler) aflagsStrings() []string {
+       aflags := util.SortFields(c.info.Aflags...)
+       return aflags
+}
+
 func (c *Compiler) lflagsString() string {
        lflags := util.SortFields(c.info.Lflags...)
        return strings.Join(lflags, " ")
 }
 
+func (c *Compiler) lflagsStrings() []string {
+       lflags := util.SortFields(c.info.Lflags...)
+       return lflags
+}
+
 func (c *Compiler) depsString() string {
        extraDeps := util.SortFields(c.extraDeps...)
        return strings.Join(extraDeps, " ") + "\n"
@@ -327,28 +359,42 @@ func (c *Compiler) depsString() string {
 // @param file                  The filename of the source file to compile.
 // @param compilerType          One of the COMPILER_TYPE_[...] constants.
 //
-// @return                      (success) The command string.
+// @return                      (success) The command arguments.
 func (c *Compiler) CompileFileCmd(file string,
-       compilerType int) (string, error) {
+       compilerType int) ([]string, error) {
 
        objFile := strings.TrimSuffix(file, filepath.Ext(file)) + ".o"
        objPath := filepath.ToSlash(c.dstDir + "/" + objFile)
 
-       var cmd string
-
+       var cmdName string
+       var flags []string
        switch compilerType {
        case COMPILER_TYPE_C:
-               cmd = c.ccPath
+               cmdName = c.ccPath
+               flags = c.cflagsStrings()
        case COMPILER_TYPE_ASM:
-               cmd = c.asPath
+               cmdName = c.asPath
+
+               // Include both the compiler flags and the assembler flags.
+               // XXX: This is not great.  We don't have a way of specifying 
compiler
+               // flags without also passing them to the assembler.
+               flags = append(c.cflagsStrings(), c.aflagsStrings()...)
        case COMPILER_TYPE_CPP:
-               cmd = c.cppPath
+               cmdName = c.cppPath
+               flags = c.cflagsStrings()
        default:
-               return "", util.NewNewtError("Unknown compiler type")
+               return nil, util.NewNewtError("Unknown compiler type")
        }
 
-       cmd += " -c " + "-o " + objPath + " " + file +
-               " " + c.cflagsString() + " " + c.includesString()
+       cmd := []string{cmdName}
+       cmd = append(cmd, flags...)
+       cmd = append(cmd, c.includesStrings()...)
+       cmd = append(cmd, []string{
+               "-c",
+               "-o",
+               objPath,
+               file,
+       }...)
 
        return cmd, nil
 }
@@ -365,23 +411,24 @@ func (c *Compiler) GenDepsForFile(file string) error {
                strings.TrimSuffix(file, filepath.Ext(file)) + ".d"
        depFile = filepath.ToSlash(depFile)
 
-       var cmd string
-       var err error
+       cmd := []string{c.ccPath}
+       cmd = append(cmd, c.cflagsStrings()...)
+       cmd = append(cmd, c.includesStrings()...)
+       cmd = append(cmd, []string{"-MM", "-MG", file}...)
 
-       cmd = c.ccPath + " " + c.cflagsString() + " " + c.includesString() +
-               " -MM -MG " + file + " > " + depFile
-       o, err := util.ShellCommand(cmd)
+       o, err := util.ShellCommandLimitDbgOutput(cmd, nil, 0)
        if err != nil {
                return util.NewNewtError(string(o))
        }
 
-       // Append the extra dependencies (.yml files) to the .d file.
-       f, err := os.OpenFile(depFile, os.O_APPEND|os.O_WRONLY, 0666)
+       // Write the compiler output to a dependency file.
+       f, err := os.OpenFile(depFile, os.O_CREATE|os.O_WRONLY, 0666)
        if err != nil {
                return util.NewNewtError(err.Error())
        }
        defer f.Close()
 
+       // Append the extra dependencies (.yml files) to the .d file.
        objFile := strings.TrimSuffix(file, filepath.Ext(file)) + ".o"
        if _, err := f.WriteString(objFile + ": " + c.depsString()); err != nil 
{
                return util.NewNewtError(err.Error())
@@ -390,16 +437,23 @@ func (c *Compiler) GenDepsForFile(file string) error {
        return nil
 }
 
+func serializeCommand(cmd []string) []byte {
+       // Use a newline as the separator rather than a space to disambiguate 
cases
+       // where arguments contain spaces.
+       return []byte(strings.Join(cmd, "\n"))
+}
+
 // Writes a file containing the command-line invocation used to generate the
 // specified file.  The file that this function writes can be used later to
 // determine if the set of compiler options has changed.
 //
 // @param dstFile               The output file whose build invocation is being
 //                                  recorded.
-// @param cmd                   The command to write.
-func writeCommandFile(dstFile string, cmd string) error {
+// @param cmd                   The command strings to write.
+func writeCommandFile(dstFile string, cmd []string) error {
        cmdPath := dstFile + ".cmd"
-       err := ioutil.WriteFile(cmdPath, []byte(cmd), 0644)
+       content := serializeCommand(cmd)
+       err := ioutil.WriteFile(cmdPath, content, 0644)
        if err != nil {
                return err
        }
@@ -448,7 +502,7 @@ func (c *Compiler) CompileFile(file string, compilerType 
int) error {
                return util.NewNewtError("Unknown compiler type")
        }
 
-       _, err = util.ShellCommand(cmd)
+       _, err = util.ShellCommand(cmd, nil)
        if err != nil {
                return err
        }
@@ -706,14 +760,13 @@ func (c *Compiler) RecursiveCompile(cType int, ignDirs 
[]string) error {
        }
 }
 
-func (c *Compiler) getObjFiles(baseObjFiles []string) string {
+func (c *Compiler) getObjFiles(baseObjFiles []string) []string {
        for objName, _ := range c.ObjPathList {
                baseObjFiles = append(baseObjFiles, objName)
        }
 
        sort.Strings(baseObjFiles)
-       objList := strings.Join(baseObjFiles, " ")
-       return objList
+       return baseObjFiles
 }
 
 // Calculates the command-line invocation necessary to link the specified elf
@@ -725,40 +778,48 @@ func (c *Compiler) getObjFiles(baseObjFiles []string) 
string {
 //                                  gets generated.
 // @param objFiles              An array of the source .o and .a filenames.
 //
-// @return                      (success) The command string.
+// @return                      (success) The command tokens.
 func (c *Compiler) CompileBinaryCmd(dstFile string, options map[string]bool,
-       objFiles []string, keepSymbols []string, elfLib string) string {
+       objFiles []string, keepSymbols []string, elfLib string) []string {
 
        objList := c.getObjFiles(util.UniqueStrings(objFiles))
 
-       cmd := c.ccPath + " -o " + dstFile + " " + " " + c.cflagsString()
+       cmd := []string{
+               c.ccPath,
+               "-o",
+               dstFile,
+       }
+       cmd = append(cmd, c.cflagsStrings()...)
 
        if elfLib != "" {
-               cmd += " -Wl,--just-symbols=" + elfLib
+               cmd = append(cmd, "-Wl,--just-symbols="+elfLib)
        }
 
        if c.ldResolveCircularDeps {
-               cmd += " -Wl,--start-group " + objList + " -Wl,--end-group "
+               cmd = append(cmd, "-Wl,--start-group")
+               cmd = append(cmd, objList...)
+               cmd = append(cmd, "-Wl,--end-group")
        } else {
-               cmd += " " + objList
+               cmd = append(cmd, objList...)
        }
 
        if keepSymbols != nil {
                for _, name := range keepSymbols {
-                       cmd += " -Wl,--undefined=" + name
+                       cmd = append(cmd, "-Wl,--undefined="+name)
                }
        }
 
-       cmd += " " + c.lflagsString()
+       cmd = append(cmd, c.lflagsStrings()...)
 
        /* so we don't get multiple global definitions of the same vartiable */
        //cmd += " -Wl,--warn-common "
 
        for _, ls := range c.LinkerScripts {
-               cmd += " -T " + ls
+               cmd = append(cmd, "-T")
+               cmd = append(cmd, ls)
        }
        if options["mapFile"] {
-               cmd += " -Wl,-Map=" + dstFile + ".map"
+               cmd = append(cmd, "-Wl,-Map="+dstFile+".map")
        }
 
        return cmd
@@ -789,7 +850,7 @@ func (c *Compiler) CompileBinary(dstFile string, options 
map[string]bool,
        }
 
        cmd := c.CompileBinaryCmd(dstFile, options, objFiles, keepSymbols, 
elfLib)
-       _, err := util.ShellCommand(cmd)
+       _, err := util.ShellCommand(cmd, nil)
        if err != nil {
                return err
        }
@@ -814,13 +875,22 @@ func (c *Compiler) CompileBinary(dstFile string, options 
map[string]bool,
 func (c *Compiler) generateExtras(elfFilename string,
        options map[string]bool) error {
 
-       var cmd string
-
        if options["binFile"] {
                binFile := elfFilename + ".bin"
-               cmd = c.ocPath + " -R .bss -R .bss.core -R .bss.core.nz -O 
binary " +
-                       elfFilename + " " + binFile
-               _, err := util.ShellCommand(cmd)
+               cmd := []string{
+                       c.ocPath,
+                       "-R",
+                       ".bss",
+                       "-R",
+                       ".bss.core",
+                       "-R",
+                       ".bss.core.nz",
+                       "-O",
+                       "binary",
+                       elfFilename,
+                       binFile,
+               }
+               _, err := util.ShellCommand(cmd, nil)
                if err != nil {
                        return err
                }
@@ -828,45 +898,71 @@ func (c *Compiler) generateExtras(elfFilename string,
 
        if options["listFile"] {
                listFile := elfFilename + ".lst"
-               // if list file exists, remove it
-               if util.NodeExist(listFile) {
-                       if err := os.RemoveAll(listFile); err != nil {
-                               return err
-                       }
+               f, err := os.OpenFile(listFile, os.O_CREATE|os.O_WRONLY, 0666)
+               if err != nil {
+                       return util.NewNewtError(err.Error())
                }
+               defer f.Close()
 
-               cmd = c.odPath + " -wxdS " + elfFilename + " >> " + listFile
-               _, err := util.ShellCommand(cmd)
+               cmd := []string{
+                       c.odPath,
+                       "-wxdS",
+                       elfFilename,
+               }
+               o, err := util.ShellCommandLimitDbgOutput(cmd, nil, 0)
                if err != nil {
                        // XXX: gobjdump appears to always crash.  Until we get 
that sorted
                        // out, don't fail the link process if lst generation 
fails.
                        return nil
                }
 
+               if _, err := f.Write(o); err != nil {
+                       return util.ChildNewtError(err)
+               }
+
                sects := []string{".text", ".rodata", ".data"}
                for _, sect := range sects {
-                       cmd = c.odPath + " -s -j " + sect + " " + elfFilename + 
" >> " +
-                               listFile
-                       util.ShellCommand(cmd)
+                       cmd := []string{
+                               c.odPath,
+                               "-s",
+                               "-j",
+                               sect,
+                               elfFilename,
+                       }
+                       o, err := util.ShellCommandLimitDbgOutput(cmd, nil, 0)
+                       if err != nil {
+                               if _, err := f.Write(o); err != nil {
+                                       return util.NewNewtError(err.Error())
+                               }
+                       }
                }
 
-               cmd = c.osPath + " " + elfFilename + " >> " + listFile
-               _, err = util.ShellCommand(cmd)
+               cmd = []string{
+                       c.osPath,
+                       elfFilename,
+               }
+               o, err = util.ShellCommandLimitDbgOutput(cmd, nil, 0)
                if err != nil {
                        return err
                }
+               if _, err := f.Write(o); err != nil {
+                       return util.NewNewtError(err.Error())
+               }
        }
 
        return nil
 }
 
 func (c *Compiler) PrintSize(elfFilename string) (string, error) {
-       cmd := c.osPath + " " + elfFilename
-       rsp, err := util.ShellCommand(cmd)
+       cmd := []string{
+               c.osPath,
+               elfFilename,
+       }
+       o, err := util.ShellCommand(cmd, nil)
        if err != nil {
                return "", err
        }
-       return string(rsp), nil
+       return string(o), nil
 }
 
 // Links the specified elf file and generates some associated artifacts (lst,
@@ -908,33 +1004,39 @@ func (c *Compiler) CompileElf(binFile string, objFiles 
[]string,
        return nil
 }
 
-func (c *Compiler) RenameSymbolsCmd(sm *symbol.SymbolMap, libraryFile string, 
ext string) string {
-       val := c.ocPath
+func (c *Compiler) RenameSymbolsCmd(
+       sm *symbol.SymbolMap, libraryFile string, ext string) []string {
 
+       cmd := []string{c.ocPath}
        for s, _ := range *sm {
-               val += " --redefine-sym " + s + "=" + s + ext
+               cmd = append(cmd, "--redefine-sym")
+               cmd = append(cmd, s+"="+s+ext)
        }
 
-       val += " " + libraryFile
-       return val
+       cmd = append(cmd, libraryFile)
+       return cmd
 }
 
-func (c *Compiler) ParseLibraryCmd(libraryFile string) string {
-       val := c.odPath + " -t " + libraryFile
-       return val
+func (c *Compiler) ParseLibraryCmd(libraryFile string) []string {
+       return []string{
+               c.odPath,
+               "-t",
+               libraryFile,
+       }
 }
 
-func (c *Compiler) CopySymbolsCmd(infile string, outfile string, sm 
*symbol.SymbolMap) string {
-
-       val := c.ocPath + " -S "
+func (c *Compiler) CopySymbolsCmd(infile string, outfile string, sm 
*symbol.SymbolMap) []string {
 
+       cmd := []string{c.ocPath, "-S"}
        for symbol, _ := range *sm {
-               val += " -K " + symbol
+               cmd = append(cmd, "-K")
+               cmd = append(cmd, symbol)
        }
 
-       val += " " + infile
-       val += " " + outfile
-       return val
+       cmd = append(cmd, infile)
+       cmd = append(cmd, outfile)
+
+       return cmd
 }
 
 // Calculates the command-line invocation necessary to archive the specified
@@ -945,10 +1047,15 @@ func (c *Compiler) CopySymbolsCmd(infile string, outfile 
string, sm *symbol.Symb
 //
 // @return                      The command string.
 func (c *Compiler) CompileArchiveCmd(archiveFile string,
-       objFiles []string) string {
+       objFiles []string) []string {
 
-       objList := c.getObjFiles(objFiles)
-       return c.arPath + " rcs " + archiveFile + " " + objList
+       cmd := []string{
+               c.arPath,
+               "rcs",
+               archiveFile,
+       }
+       cmd = append(cmd, c.getObjFiles(objFiles)...)
+       return cmd
 }
 
 func linkerScriptFileName(archiveFile string) string {
@@ -1027,18 +1134,18 @@ func (c *Compiler) CompileArchive(archiveFile string) 
error {
        util.StatusMessage(util.VERBOSITY_DEFAULT, "Archiving %s\n",
                path.Base(archiveFile))
        objList := c.getObjFiles([]string{})
-       if objList == "" {
+       if objList == nil {
                return nil
        }
        util.StatusMessage(util.VERBOSITY_VERBOSE, "Archiving %s with object "+
-               "files %s\n", archiveFile, objList)
+               "files %s\n", archiveFile, strings.Join(objList, " "))
 
        if err != nil && !os.IsNotExist(err) {
                return util.NewNewtError(err.Error())
        }
 
        cmd := c.CompileArchiveCmd(archiveFile, objFiles)
-       _, err = util.ShellCommand(cmd)
+       _, err = util.ShellCommand(cmd, nil)
        if err != nil {
                return err
        }
@@ -1125,7 +1232,7 @@ func (c *Compiler) RenameSymbols(sm *symbol.SymbolMap, 
libraryFile string, ext s
 
        cmd := c.RenameSymbolsCmd(sm, libraryFile, ext)
 
-       _, err := util.ShellCommand(cmd)
+       _, err := util.ShellCommand(cmd, nil)
 
        return err
 }
@@ -1133,7 +1240,7 @@ func (c *Compiler) RenameSymbols(sm *symbol.SymbolMap, 
libraryFile string, ext s
 func (c *Compiler) ParseLibrary(libraryFile string) (error, []byte) {
        cmd := c.ParseLibraryCmd(libraryFile)
 
-       out, err := util.ShellCommand(cmd)
+       out, err := util.ShellCommand(cmd, nil)
        if err != nil {
                return err, nil
        }
@@ -1143,7 +1250,7 @@ func (c *Compiler) ParseLibrary(libraryFile string) 
(error, []byte) {
 func (c *Compiler) CopySymbols(infile string, outfile string, sm 
*symbol.SymbolMap) error {
        cmd := c.CopySymbolsCmd(infile, outfile, sm)
 
-       _, err := util.ShellCommand(cmd)
+       _, err := util.ShellCommand(cmd, nil)
        if err != nil {
                return err
        }

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-newt/blob/0241d2f3/newt/toolchain/deps.go
----------------------------------------------------------------------
diff --git a/newt/toolchain/deps.go b/newt/toolchain/deps.go
index 86ec2cc..a8c7f97 100644
--- a/newt/toolchain/deps.go
+++ b/newt/toolchain/deps.go
@@ -57,8 +57,7 @@ func parseDepsLine(line string) (string, []string, error) {
 
        dFileTok := tokens[0]
        if dFileTok[len(dFileTok)-1:] != ":" {
-               return "", nil, util.NewNewtError("Invalid Makefile dependency 
file; " +
-                       "line missing ':'")
+               return "", nil, util.NewNewtError("line missing ':'")
        }
 
        dFileName := dFileTok[:len(dFileTok)-1]
@@ -91,7 +90,9 @@ func ParseDepsFile(filename string) ([]string, error) {
        for _, line := range lines {
                src, deps, err := parseDepsLine(line)
                if err != nil {
-                       return nil, err
+                       return nil, util.FmtNewtError(
+                               "Invalid Makefile dependency file \"%s\"; %s",
+                               filename, err.Error())
                }
 
                if dFile == "" {
@@ -133,14 +134,16 @@ func (tracker *DepTracker) ProcessFileTime(file string) 
error {
 // @return                      true if the command has changed or if the
 //                                  destination file was never built;
 //                              false otherwise.
-func commandHasChanged(dstFile string, cmd string) bool {
+func commandHasChanged(dstFile string, cmd []string) bool {
        cmdFile := dstFile + ".cmd"
        prevCmd, err := ioutil.ReadFile(cmdFile)
        if err != nil {
                return true
        }
 
-       return bytes.Compare(prevCmd, []byte(cmd)) != 0
+       curCmd := serializeCommand(cmd)
+
+       return bytes.Compare(prevCmd, curCmd) != 0
 }
 
 // Determines if the specified C or assembly file needs to be built.  A compile

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-newt/blob/0241d2f3/newt/vendor/mynewt.apache.org/newt/util/util.go
----------------------------------------------------------------------
diff --git a/newt/vendor/mynewt.apache.org/newt/util/util.go 
b/newt/vendor/mynewt.apache.org/newt/util/util.go
index a1b85f2..37902b5 100644
--- a/newt/vendor/mynewt.apache.org/newt/util/util.go
+++ b/newt/vendor/mynewt.apache.org/newt/util/util.go
@@ -268,13 +268,47 @@ func ReadConfig(path string, name string) (*viper.Viper, 
error) {
        }
 }
 
-// Execute the command specified by cmdStr on the shell and return results
-func ShellCommand(cmdStr string) ([]byte, error) {
-       log.Debug(cmdStr)
-       cmd := exec.Command("sh", "-c", cmdStr)
+// Execute the specified process and block until it completes.  Additionally,
+// the amount of combined stdout+stderr output to be logged to the debug log
+// can be restricted to a maximum number of characters.
+//
+// @param cmdStrs               The "argv" strings of the command to execute.
+// @param env                   Additional key=value pairs to inject into the
+//                                  child process's environment.  Specify null
+//                                  to just inherit the parent environment.
+// @param maxDbgOutputChrs      The maximum number of combined stdout+stderr
+//                                  characters to write to the debug log.
+//                                  Specify -1 for no limit; 0 for no output.
+//
+// @return []byte               Combined stdout and stderr output of process.
+// @return error                NewtError on failure.
+func ShellCommandLimitDbgOutput(
+       cmdStrs []string, env []string, maxDbgOutputChrs int) ([]byte, error) {
+
+       envLogStr := ""
+       if env != nil {
+               envLogStr = strings.Join(env, " ") + " "
+       }
+       log.Debugf("%s%s", envLogStr, strings.Join(cmdStrs, " "))
+
+       name := cmdStrs[0]
+       args := cmdStrs[1:]
+       cmd := exec.Command(name, args...)
+
+       if env != nil {
+               cmd.Env = append(env, os.Environ()...)
+       }
 
        o, err := cmd.CombinedOutput()
-       log.Debugf("o=%s", string(o))
+
+       if maxDbgOutputChrs < 0 || len(o) <= maxDbgOutputChrs {
+               dbgStr := string(o)
+               log.Debugf("o=%s", dbgStr)
+       } else if maxDbgOutputChrs != 0 {
+               dbgStr := string(o[:maxDbgOutputChrs]) + "[...]"
+               log.Debugf("o=%s", dbgStr)
+       }
+
        if err != nil {
                return o, NewNewtError(string(o))
        } else {
@@ -282,6 +316,19 @@ func ShellCommand(cmdStr string) ([]byte, error) {
        }
 }
 
+// Execute the specified process and block until it completes.
+//
+// @param cmdStrs               The "argv" strings of the command to execute.
+// @param env                   Additional key=value pairs to inject into the
+//                                  child process's environment.  Specify null
+//                                  to just inherit the parent environment.
+//
+// @return []byte               Combined stdout and stderr output of process.
+// @return error                NewtError on failure.
+func ShellCommand(cmdStrs []string, env []string) ([]byte, error) {
+       return ShellCommandLimitDbgOutput(cmdStrs, env, -1)
+}
+
 // Run interactive shell command
 func ShellInteractiveCommand(cmdStr []string, env []string) error {
        log.Print("[VERBOSE] " + cmdStr[0])
@@ -297,7 +344,10 @@ func ShellInteractiveCommand(cmdStr []string, env 
[]string) error {
                <-c
        }()
 
-       env = append(env, os.Environ()...)
+       if env != nil {
+               env = append(env, os.Environ()...)
+       }
+
        // Transfer stdin, stdout, and stderr to the new process
        // and also set target directory for the shell to start in.
        // and set the additional environment variables

http://git-wip-us.apache.org/repos/asf/incubator-mynewt-newt/blob/0241d2f3/util/util.go
----------------------------------------------------------------------
diff --git a/util/util.go b/util/util.go
index a1b85f2..37902b5 100644
--- a/util/util.go
+++ b/util/util.go
@@ -268,13 +268,47 @@ func ReadConfig(path string, name string) (*viper.Viper, 
error) {
        }
 }
 
-// Execute the command specified by cmdStr on the shell and return results
-func ShellCommand(cmdStr string) ([]byte, error) {
-       log.Debug(cmdStr)
-       cmd := exec.Command("sh", "-c", cmdStr)
+// Execute the specified process and block until it completes.  Additionally,
+// the amount of combined stdout+stderr output to be logged to the debug log
+// can be restricted to a maximum number of characters.
+//
+// @param cmdStrs               The "argv" strings of the command to execute.
+// @param env                   Additional key=value pairs to inject into the
+//                                  child process's environment.  Specify null
+//                                  to just inherit the parent environment.
+// @param maxDbgOutputChrs      The maximum number of combined stdout+stderr
+//                                  characters to write to the debug log.
+//                                  Specify -1 for no limit; 0 for no output.
+//
+// @return []byte               Combined stdout and stderr output of process.
+// @return error                NewtError on failure.
+func ShellCommandLimitDbgOutput(
+       cmdStrs []string, env []string, maxDbgOutputChrs int) ([]byte, error) {
+
+       envLogStr := ""
+       if env != nil {
+               envLogStr = strings.Join(env, " ") + " "
+       }
+       log.Debugf("%s%s", envLogStr, strings.Join(cmdStrs, " "))
+
+       name := cmdStrs[0]
+       args := cmdStrs[1:]
+       cmd := exec.Command(name, args...)
+
+       if env != nil {
+               cmd.Env = append(env, os.Environ()...)
+       }
 
        o, err := cmd.CombinedOutput()
-       log.Debugf("o=%s", string(o))
+
+       if maxDbgOutputChrs < 0 || len(o) <= maxDbgOutputChrs {
+               dbgStr := string(o)
+               log.Debugf("o=%s", dbgStr)
+       } else if maxDbgOutputChrs != 0 {
+               dbgStr := string(o[:maxDbgOutputChrs]) + "[...]"
+               log.Debugf("o=%s", dbgStr)
+       }
+
        if err != nil {
                return o, NewNewtError(string(o))
        } else {
@@ -282,6 +316,19 @@ func ShellCommand(cmdStr string) ([]byte, error) {
        }
 }
 
+// Execute the specified process and block until it completes.
+//
+// @param cmdStrs               The "argv" strings of the command to execute.
+// @param env                   Additional key=value pairs to inject into the
+//                                  child process's environment.  Specify null
+//                                  to just inherit the parent environment.
+//
+// @return []byte               Combined stdout and stderr output of process.
+// @return error                NewtError on failure.
+func ShellCommand(cmdStrs []string, env []string) ([]byte, error) {
+       return ShellCommandLimitDbgOutput(cmdStrs, env, -1)
+}
+
 // Run interactive shell command
 func ShellInteractiveCommand(cmdStr []string, env []string) error {
        log.Print("[VERBOSE] " + cmdStr[0])
@@ -297,7 +344,10 @@ func ShellInteractiveCommand(cmdStr []string, env 
[]string) error {
                <-c
        }()
 
-       env = append(env, os.Environ()...)
+       if env != nil {
+               env = append(env, os.Environ()...)
+       }
+
        // Transfer stdin, stdout, and stderr to the new process
        // and also set target directory for the shell to start in.
        // and set the additional environment variables

Reply via email to