[jira] [Commented] (MYNEWT-365) newt can't handle files with spaces
[ https://issues.apache.org/jira/browse/MYNEWT-365?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15712904#comment-15712904 ] ASF subversion and git services commented on MYNEWT-365: Commit 0241d2f38e8236f03d6f20d5c7198d4a0a4cb169 in incubator-mynewt-newt's branch refs/heads/develop from [~ccollins476] [ https://git-wip-us.apache.org/repos/asf?p=incubator-mynewt-newt.git;h=0241d2f ] 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. > newt can't handle files with spaces > --- > > Key: MYNEWT-365 > URL: https://issues.apache.org/jira/browse/MYNEWT-365 > Project: Mynewt > Issue Type: Bug > Components: Newt >Affects Versions: v0_9_0 > Environment: Any >Reporter: Tim >Assignee: Christopher Collins >Priority: Minor > > I was just looking at the code that decides where gcc is run from (so I can > see if I can make it always run from /workspace, so full paths are used), > when I noticed you don't handle program arguments properly. > See [this > code|https://github.com/apache/incubator-mynewt-newt/blob/8abc4f2c2bb1ab784a854cdf5662e79d88a41407/newt/toolchain/compiler.go#L262] > {code} > cmd += " -c " + "-o " + objPath + " " + file + > " " + c.cflagsString() + " " + c.includesString() > {code} > Command line arguments should be stored as a {{[]string}} instead of a > space-delimited {{string}}. Then you don't need to worry about spaces or > escaping or anything like that. In other words something like this: > {code} > func (c *Compiler) CompileFileCmd(file string, > compilerType int) ([]string, error) { > objFile := strings.TrimSuffix(file, filepath.Ext(file)) + ".o" > objPath := filepath.ToSlash(c.dstDir + "/" + objFile) > cmd := make([]string) > switch compilerType { > case COMPILER_TYPE_C: > cmd = cmd.append(c.ccPath) > case COMPILER_TYPE_ASM: > cmd = cmd.append(c.asPath) > default: > return nil, util.NewNewtError("Unknown compiler type") > } > cmd = append(cmd, "-c", "-o", objPath, file) > // There will be some special handling for these, depending on what > they contain (I'm not sure of the format of these exactly). > //c.cflagsString(), c.includesString() > return cmd, nil > } > {code} > And then don't use ShellCommand() to run it. Is there any reason that you're > using {{sh -c}} rather than just running the command directly? It's going to > make porting to Windows a pain. Similarly for code like this: > {code} > func CopyFile(srcFile string, destFile string) error { > _, err := ShellCommand(fmt.Sprintf("mkdir -p %s", > filepath.Dir(destFile))) > if err != nil { > return err > } > if _, err := ShellCommand(fmt.Sprintf("cp -Rf %s %s", srcFile, > destFile)); err != nil { > return err > } > return nil > } > {code} > That won't work on Windows and also won't work with files containing spaces. > Better to use Go's proper functions for creating directories and copying > files. (Unfortunately there isn't a built-in equivalent of {{cp -Rf}} but if > you google it there is lots of example code.) -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (MYNEWT-365) newt can't handle files with spaces
[ https://issues.apache.org/jira/browse/MYNEWT-365?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15712906#comment-15712906 ] ASF subversion and git services commented on MYNEWT-365: Commit 5c5a987ef72ae9b3eb682f19d7d0bdc85f7e500c in incubator-mynewt-core's branch refs/heads/develop from [~ccollins476] [ https://git-wip-us.apache.org/repos/asf?p=incubator-mynewt-core.git;h=5c5a987 ] MYNEWT-365 newt can't handle files with spaces This ticket's sister commit changed newt in a way that breaks compatibility with the core repo. This commit makes the core repo compatible with newt again. 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. > newt can't handle files with spaces > --- > > Key: MYNEWT-365 > URL: https://issues.apache.org/jira/browse/MYNEWT-365 > Project: Mynewt > Issue Type: Bug > Components: Newt >Affects Versions: v0_9_0 > Environment: Any >Reporter: Tim >Assignee: Christopher Collins >Priority: Minor > > I was just looking at the code that decides where gcc is run from (so I can > see if I can make it always run from /workspace, so full paths are used), > when I noticed you don't handle program arguments properly. > See [this > code|https://github.com/apache/incubator-mynewt-newt/blob/8abc4f2c2bb1ab784a854cdf5662e79d88a41407/newt/toolchain/compiler.go#L262] > {code} > cmd += " -c " + "-o " + objPath + " " + file + > " " + c.cflagsString() + " " + c.includesString() > {code} > Command line arguments should be stored as a {{[]string}} instead of a > space-delimited {{string}}. Then you don't need to worry about spaces or > escaping or anything like that. In other words something like this: > {code} > func (c *Compiler) CompileFileCmd(file string, > compilerType int) ([]string, error) { > objFile := strings.TrimSuffix(file, filepath.Ext(file)) + ".o" > objPath := filepath.ToSlash(c.dstDir + "/" + objFile) > cmd := make([]string) > switch compilerType { > case COMPILER_TYPE_C: > cmd = cmd.append(c.ccPath) > case COMPILER_TYPE_ASM: > cmd = cmd.append(c.asPath) > default: > return nil, util.NewNewtError("Unknown compiler type") > } > cmd = append(cmd, "-c", "-o", objPath, file) > // There will be some special handling for these, depending on what > they contain (I'm not sure of the format of these exactly). > //c.cflagsString(), c.includesString() > return cmd, nil > } > {code} > And then don't use ShellCommand() to run it. Is there any reason that you're > using {{sh -c}} rather than just running the command directly? It's going to > make porting to Windows a pain. Similarly for code like this: > {code} > func CopyFile(srcFile string, destFile string) error { > _, err := ShellCommand(fmt.Sprintf("mkdir -p %s", > filepath.Dir(destFile))) > if err != nil { > return err > } > if _, err := ShellCommand(fmt.Sprintf("cp -Rf %s %s", srcFile, > destFile)); err != nil { > return err > } > return nil > } > {code} > That won't work on Windows and also won't work with files containing spaces. > Better to use Go's proper functions for creating directories and copying > files. (Unfortunately there isn't a built-in equivalent of {{cp -Rf}} but if > you google it there is lots of example code.) -- This message was sent by Atlassian JIRA (v6.3.4#6332)