Support bash hooks on windows #636
Conversation
|
Nice!! |
lox
force-pushed
the
support-bash-hooks-on-windows
branch
2 times, most recently
from
db1fa1e
to
692cd94
Compare
|
Rad! Would this mean our plugins "just work" for the most part then? |
Yeah, hopefully :) |
# Executing "C:\Users\ContainerAdministrator\AppData\Local\Temp\buildkite-agent-bootstrap-hook-runner-066954642"
--
| > C:\Program Files\Git\usr\bin\bash.exe -c C:/Users/ContainerAdministrator/AppData/Local/Temp/buildkite-agent-bootstrap-hook-runner-066954642
| This is an example of a pre-command hook from .buildkite/hooks/pre-command |
|
I'm struggling with this error: Any ideas @keithpitt? |
|
Ah, right. It seems to be mistakenly detecting that there is a STDIN. |
bootstrap/bootstrap.go
Outdated
| func (b *Bootstrap) findHookFile(hookDir string, name string) (string, error) { | ||
| if runtime.GOOS == "windows" { | ||
| // check for windows types first | ||
| if p, err := shell.LookPath(name, hookDir, ".BAT;.CMD;.EXE"); err == nil { |
There was a problem hiding this comment.
Unfortunately we can't support powershell hooks yet, as need to write a environment hook magic thing for them.
bootstrap/bootstrap.go
Outdated
| func dirForAgentName(agentName string) string { | ||
| badCharsPattern := regexp.MustCompile("[[:^alnum:]]") | ||
| return badCharsPattern.ReplaceAllString(agentName, "-") | ||
| } | ||
|
|
||
| func buildkiteAgentBinary() string { |
There was a problem hiding this comment.
This replaces hardcoded buildkite-agent
lox
force-pushed
the
support-bash-hooks-on-windows
branch
from
b9f5db1
to
820ba8d
Compare
Replaced with golang.org/x/crypto/ssh/terminal which seems to work better.
lox
force-pushed
the
support-bash-hooks-on-windows
branch
from
820ba8d
to
33697a1
Compare
lox
force-pushed
the
support-bash-hooks-on-windows
branch
from
d35f0e8
to
59cdbc9
Compare
| @@ -108,7 +108,7 @@ var AnnotateCommand = cli.Command{ | |||
|
|
|||
| if cfg.Body != "" { | |||
| body = cfg.Body | |||
| } else if stdin.IsPipe() { | |||
| } else if terminal.IsTerminal(int(os.Stdin.Fd())) { | |||
There was a problem hiding this comment.
Ah, is this the cross platform way to do it?
There was a problem hiding this comment.
Yup, or at least it let's it be someone else's problem. Our implementation didn't seem to work on Windows, despite it saying it did.
| @@ -97,12 +105,11 @@ func newHookScriptWrapper(hookPath string) (*hookScriptWrapper, error) { | |||
| "SET > \"" + h.afterEnvFile.Name() + "\"\n" + | |||
| "EXIT %" + hookExitStatusEnv + "%" | |||
| } else { | |||
| script = "#!/bin/bash\n" + | |||
There was a problem hiding this comment.
I ditched this shebang, we always invoke these scripts with an interpreter and it was causing problems on windows bash.
@keithpitt had a neat suggestion for how to support plugin hooks on windows, which was to use the bash.exe bundled with Git for Windows. This generalizes that and searches for the best option under windows, either batch, powershell or if neither of those exist it uses the bash/sh options.