skelterjohn/go-gb

weekly update 2011-06-02 breaks gb

kortschak opened this issue · 5 comments

Changes to the exec package break gb in runext: RunExternal and RunExternalDump. I've made what I think are reasonable changes to correct this, but cannot get gb to correctly build a previously function project.

http://gist.github.com/1013798

Either that link doesn't work or I don't know how to use it.

I'm fixing gb now, though. Thanks for the note.

I've got a fix, but I'm having trouble pushing it. It will get in tonight, at the latest.

I still didn't see your patch, but perhaps you weren't stripping the first element out of the command line args? The old api wanted the arg list like you'd see in os.Args, where the first element is the short name for the executable running. The new api doesn't want it.

My attempt at a patch (some untested hand modification of the patch in
the wmsg assertion - appologies if this fails):

gist URL fixed - sorry.

diff --git a/gb/runext.go b/gb/runext.go
index a5a2ab1..6ecb46c 100644
--- a/gb/runext.go
+++ b/gb/runext.go
@@ -21,6 +21,7 @@ import (
"exec"
"fmt"
"strings"

  • "bytes"
    )

var MakeCMD,
@@ -100,45 +101,28 @@ func SplitArgs(args []string) (sargs []string) {

func RunExternalDump(cmd, wd string, argv []string, dump *os.File) (err os.Error) {
argv = SplitArgs(argv)

  • var p *exec.Cmd
  • p, err = exec.Run(cmd, argv, os.Environ(), wd, exec.PassThrough, exec.Pipe, exec.PassThrough)
  • if err != nil {
  •   return
    
  • }
  • if p != nil {

- src := p.Stdout

  •   io.Copy(dump, src)
    
  • if p := exec.Command(argv[0], argv[1:]...); p != nil {
  •   var src []byte
    
  •   src, err = p.Output()
    
  •   buf := bytes.NewBuffer(src)
    
  •   io.Copy(dump, buf)
    
  •   var wmsg *os.Waitmsg
    
  •   wmsg, err = p.Wait(0)
    
  •   if wmsg.ExitStatus() != 0 {
    
  •       err = os.NewError(fmt.Sprintf("%v: %s\n", argv, wmsg.String()))
    
  •       return
    
  •   }
    
  •   if err != nil {
    
  •       return
    
  •   if wmsg, ok := err.(os.WaitMsg); ok {
    
  •       err = os.NewError(fmt.Sprintf("%v: %s\n", argv, wmsg.String()))
    }
    
    }
    return
    }
    func RunExternal(cmd, wd string, argv []string) (err os.Error) {
    argv = SplitArgs(argv)
  • var p *exec.Cmd
  • p, err = exec.Run(cmd, argv, os.Environ(), wd, exec.PassThrough, exec.PassThrough, exec.PassThrough)
  • if err != nil {
  •   return
    
  • }
  • if p != nil {
  •   var wmsg *os.Waitmsg
    
  •   wmsg, err = p.Wait(0)
    
  •   if wmsg.ExitStatus() != 0 {
    
  •       err = os.NewError(fmt.Sprintf("%v: %s\n", argv, wmsg.String()))
    
  •       return
    
  •   }
    
  •   if err != nil {
    
  •       return
    
  • if p := exec.Command(argv[0], argv[1:]...); p != nil {
  •   err = p.Run()
    
  •   if wmsg, ok := err.(os.WaitMesg); ok {
    
  •       err = os.NewError(fmt.Sprintf("%v: %s\n", argv, wmsg.String()))
    }
    

    }
    return
    }
    +

    1.7.0.4

There also appears to be some bad interaction with GOPATH. If it's set
gb fails, trying to ascend the file system tree beyond root.

The fix above allowed me to at least get a compiler failing error, but
on code that I know should compile and without and indication of what
caused the failure.

cheers

On Wed, 2011-06-08 at 07:50 -0700, skelterjohn wrote:

I've got a fix, but I'm having trouble pushing it. It will get in
tonight, at the latest.

I still didn't see your patch, but perhaps you weren't stripping the
first element out of the command line args? The old api wanted the arg
list like you'd see in os.Args, where the first element is the short
name for the executable running. The new api doesn't want it.

Please file another issue for the gopath thing, if it persists when you update (just pushed the fix for this issue).

Looks like the GOPATH problem was me not interacting with GOPATH properly.