Skip to content

Commit

Permalink
Windows fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
@meteorcloudy
meteorcloudy committed Oct 25, 2018
1 parent 97aee99 commit ea225fe
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 2 deletions.
7 changes: 6 additions & 1 deletion ibazel/command/command.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"io/ioutil"
"os"
"os/exec"
"runtime"

"github.com/bazelbuild/bazel-watcher/bazel"
"github.com/bazelbuild/bazel-watcher/ibazel/process_group"
Expand All @@ -40,7 +41,11 @@ type Command interface {
// start will be called by most implementations since this logic is extremely
// common.
func start(b bazel.Bazel, target string, args []string) (*bytes.Buffer, process_group.ProcessGroup) {
tmpfile, err := ioutil.TempFile("", "bazel_script_path")
suffix := ""
if runtime.GOOS == "windows" {
suffix = ".bat"
}
tmpfile, err := ioutil.TempFile("", "bazel_script_path.*" + suffix)
if err != nil {
fmt.Print(err)
}
Expand Down
2 changes: 1 addition & 1 deletion tools/bazel.rc
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
build --workspace_status_command=tools/workplace_status.sh
build --workspace_status_command=tools/workplace_status.bat

# Flags used during releases
build:release --stamp
Expand Down

3 comments on commit ea225fe

@achew22
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These all look very reasonable. Do you know what the plan is to set workspace status command have a Windows value and a non Windows value? Is it possible, for example, for that to be a target in the repository? Then we could use select to figure out the correct files to run

@meteorcloudy
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, you cannot use select on --workspace_status_command. The best I can think of that would work on both platform is using a python script. eg. --workspace_status_command="python tools/worksplace_status.py"

@achew22
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we know that python is going to be available on all systems? I would hate to rewrite it into python only to find out that adds a hard dependency on python that was otherwise unnecessary.

Please sign in to comment.