Windows updates failing

[bgentry]

>hk update
rename hk.part .\hk.exe: Cannot create a file when that file already exists.

[bgentry]

I reworked this to rename the running executable before moving the new one into its place. According to this answer and go-update, that should work.

However, I'm seeing this as of d207814 / v20130917.10 when updating to v20130917.11 (same commit):

>hk update
rename .\hk.exe .\hk.exe.old: The process cannot access the file because it is being used by another process.

It looks like I'm doing everything as go-update does... I wonder if this is an artifact of cross-compiling?

[bgentry]

These are the only differences I can see from the go-update approach:

• To write the new executable, we're using os.FileCopy() and go-update is using os.OpenFile() + io.Copy. This shouldn't matter though since the problem we're having is renaming the old exe while it's running
• go-update uses the full file path (via go-execpath) for referencing the running executable. As you can see from the error above, our method just yields the relative path \hk.exe
• I'm using cross-compiled binaries, while go-update is known to work with natively compiled binaries. Though I haven't been able to find any relevant differences relating to how os.Rename() works on Windows, it's possible there's something I haven't found yet.

The author of go-update was kind enough to email me and confirm that his approach does in fact work on Windows XP and Windows 2008 Server. I'm testing on Windows 8, so that should be the same.

The one caveat he mentioned that we'll need to consider is that, on Windows, you cannot delete the old binary while it's still running. As such, we'll get left with a hk.exe.old file that we'd need to clean up before running another update.

[inconshreveable]

My pull request is untested so you should test it before you merge it in. That said, the fix I'm applying is the same that I've already applied to fix the exact same issue in go-update.

Speaking of which, @bgentry , @kr :

I would love to consolidate effort and make go-update generic enough for use across projects. What flexibility/APIs would you need to make that a viable option?

[bgentry]

@inconshreveable awesome! I will test it shortly. I know one concern we had was with go-execpath and the way it can't be cross-compiled due to cgo dependencies for GetNative().

@ddollar did point us at osext today which appears to work great without the cgo dependency. Maybe take a look at it and see if it obviates the need for go-execpath?

I'd certainly love to help maintain a common lib for this functionality rather than duplicating it.

[inconshreveable]

@bgentry Yeah, I got in touch with the author of osext the other day to ask about osext's method of getting the path on darwin. It's using a completely undocumented syscall that gets the process arguments and then does path combining in some cases. It worries me that this would suffer from the same issues that consulting Arg0 normally does, but I don't have a darwin box to test with.

[bgentry]

@inconshreveable thanks for #63, that did work:

C:\Users\bgentry\Desktop>hk version
20130918

C:\Users\bgentry\Desktop>hk update

C:\Users\bgentry\Desktop>hk version
20130918.1

Also still works on OSX:

$ hk version
20130918
$ hk update
$ hk version
20130918.1

I do still need to add a line to remove hk.exe.old cleanly though.. How were you planning to handle that part on go-update? Just clear the file before beginning the update process? Or somehow fork & clean up after the update?

I'll test out osext locally on darwin and let you know what I get back from it.

[bgentry]

Tried this locally on darwin kernel 12.4.0 / OSX 10.8.4:

package main

import (
    "bitbucket.org/kardianos/osext"
    "fmt"
)

func main() {
    path, err := osext.Executable()
    if err != nil {
        fmt.Printf("Error: %q\n", err)
    } else {
        fmt.Printf("Path: %q\n", path)
    }
}

Then did a go install on that app to put its bin in my PATH at /usr/local/bin. Appears to work great:

~ $ osexttest 
Path: "/usr/local/bin/osexttest"
~ $ /usr/local/bin/osexttest 
Path: "/usr/local/bin/osexttest"
$ mv /usr/local/bin/osexttest /usr/local/bin/osext\ test 
$ /usr/local/bin/osext\ test 
Path: "/usr/local/bin/osext test"

Is there another scenario I need to test?

I guess the remaining concern is around how volatile that undocumented syscall is to fetch the path. I don't have any knowledge around that and I don't even know who I'd ask about it :(

[bgentry]

Also tried invoking via another script (test.go) with a weird argv[0]:

package main

import (
    "fmt"
    "os/exec"
)

func main() {
    cmd := exec.Command("./.osexttest")
    cmd.Args[0] = "foo"
    str, err := cmd.Output()
    if err != nil {
        fmt.Println("Error:", err)
    }
    fmt.Println(string(str))
}

test.go was in the same dir as the executable (renamed .osexttest for this), and running it yielded the correct result:

$ go run test.go 
Path: "/Users/bgentry/go/src/osexttest/.osexttest"

[bgentry]

Seems the only issue with this mechanism is that it won't work for symlinks: http://unixjunkie.blogspot.com/2006/10/update-char-apple-argument-vector.html

@inconshreveable since you've been in contact with the author, maybe mention that he'd need to use filepath.EvalSymlinks() to handle that case, at least on OSX?

Other than that it looks like a pretty solid lib.

[bgentry]

Marking this issue as closed for tracking purposes, but I'm happy to continue the conversation about what we'd need to converge on go-update :)

[inconshreveable]

@bgentry thanks for testing that out! I'll mention the symlink problem to the author and then I'll deprecate go-execpath in favor of osext. I'll also open an issue on go-update to continue the discussion on code reuse.