Windows support

[xialvjun]

file modd/test/escaping/a"b make windows git checkout failed

[cortesi]

At this point modd doesn't support Windows. I'd love to do it, but it's going to be slightly tricky, and Windows is not my primary development platform. I'd need someone willing to help with this (and to help support it down the track as well).

If we plan to support it, the tests should be refactored to create these files dynamically at test time in a temporary directory, and we should disable files Windows can't handle based on the platform.

There are a number of other issues here for Windows.

• Lack of support for signals - we'd have to reduce the signal set to the couple of pseudo-signals Go supports on Windows.
• Communication with devd relies on signals on Unixen, and signals are missing on Windows. We'd have to find some other way for modd to signal to devd that it should trigger a livereload.
• Lastly, modd has standardised on bash for its command execution, and bash obviously doesn't exist on Windows. We'd have to do something else here - perhaps powershell?
• Finally, because of the signals and scripting differences, modd on Windows would be a different animal than modd on unix, and modd.conf files across the two flavors would not be compatible. We'd have to decide how we want to handle that, and in particular how we could support projects that have developers using both Windows and Unixes.

[cortesi]

I've changed the title of this ticket to "Windows support" so it can collect all these issues in one place.

[cortesi]

Most of this was done in one fell swoop in 6fbfba0520b5b00f616e647a0be4a36787ad1301. The next release - probably out tomorrow - will support Windows.

[kardianos]

What's the rational for requiring sh or bash? If just for quoting strings, you could use: https://godoc.org/github.com/google/shlex

If bash and sh aren't found, could we just drop back to something like the above shlex and run the command with exec.Command directly on Windows? (I can drop a PR if you think that sounds reasonable)

[kardianos]

Alternatively, the following could work when bash and sh aren't present:

diff --git a/daemon.go b/daemon.go
index efe7a26..93a7328 100644
--- a/daemon.go
+++ b/daemon.go
@@ -32,13 +32,13 @@ func (d *daemon) Run() {
            time.Sleep(MinRestart - since)
        }
        lastStart = time.Now()
-       sh, err := getShell()
+       sh, arg, err := getShell()
        if err != nil {
            d.log.Shout("%s", err)
            return
        }

-       c := exec.Command(sh, "-c", d.conf.Command)
+       c := exec.Command(sh, arg, d.conf.Command)
        stdo, err := c.StdoutPipe()
        if err != nil {
            d.log.Shout("%s", err)
diff --git a/prep.go b/prep.go
index 42c5814..9723dad 100644
--- a/prep.go
+++ b/prep.go
@@ -25,11 +25,11 @@ func (p ProcError) Error() string {
 // RunProc runs a process to completion, sending output to log
 func RunProc(cmd string, log termlog.Stream) error {
    log.Header()
-   sh, error := getShell()
+   sh, arg, error := getShell()
    if error != nil {
        return error
    }
-   c := exec.Command(sh, "-c", cmd)
+   c := exec.Command(sh, arg, cmd)
    stdo, err := c.StdoutPipe()
    if err != nil {
        return err
diff --git a/proc.go b/proc.go
index 26f24b0..4dd9acf 100644
--- a/proc.go
+++ b/proc.go
@@ -11,14 +11,17 @@ import (
    "github.com/cortesi/termlog"
 )

-func getShell() (string, error) {
+func getShell() (string, string, error) {
    if _, err := exec.LookPath("bash"); err == nil {
-       return "bash", nil
+       return "bash", "-c", nil
    }
    if _, err := exec.LookPath("sh"); err == nil {
-       return "sh", nil
+       return "sh", "-c", nil
    }
-   return "", fmt.Errorf("Could not find bash or sh on path.")
+   if _, err := exec.LookPath("cmd.exe"); err == nil {
+       return "cmd.exe", "/c", nil
+   }
+   return "", "", fmt.Errorf("Could not find bash or sh on path.")
 }

 // shortCommand shortens a command to a name we can use in a notification

[cortesi]

Hi there. I'm not wedded to the sh/bash dependency, but I do think there are some good reasons to stick with it.

• One is that I want people to be able to check modd.conf files into their repos, and be reasonably certain that they will act the same way for all (or nearly all) users. Allowing sh as a fallback is already a bit of a compromise here - just mandating one single shell (either bash or POSIX sh) was my first instinct.
• A second reason is that having commands be small embedded shell scripts lets me avoid implementing a host of features that I would otherwise have to design into modd. For instance, to send a desktop alert you can just redirect output to stderr and exit with an error using "false". So modd really relies on a rich shell environment being there. That being the case, I think it makes sense to pick one and stick with it.
• Some modd features rely on knowing how the executing shell will behave. For instance, the quoting behaviours need to be understood to make the mods and dirmods variables work right. I had some issues with cmd.exe in this respect that would need to be sorted out.

So, all in all, I think installing a bash/sh is not too much effort on Windows, and it's probably OK to rely on it. Happy to hear suggestions for how this can be improved.

[kardianos]

Okay, I might need to fork or create/use a different watcher.

My perspective is

• Portability: because bash is often an alias for different shells, dash, sh, etc, and because systems have different versions or no bash at all, it is exactly for that reason that you wouldn't shell out. You are correct about cmd escaping issues, it uses a two quotes to escape to one quote, rather than a backslash quote.
• I understand not wanting to re-implement lots of features. But that desktop alert function sounds like an OS X feature, not a Linux or windows feature (I use Linux and Windows), thus the above portability issue.
• I agree on the last point, having different shells is kinda a pain. But... really all I want to do is to execute a full executable when something changes, not actually run a shell.

I have bash installed, but not in my PATH, per my design. My coworkers won't even have bash installed.

I think we agree it should be portable, you would advocate depending on a consistent bash being present, I would advocate building any desired functionality in. Either looks valid I suppose. If you're set on the bash dep, I'll probably make my own. Thanks for the response!

[cortesi]

Well whether you start your own project or not is obviously up to you. As I said, I'm happy to discuss changes to the way modd works if they're well motivated.

The fact that bash is aliased to different shells is a non-issue - all these shells are bash compatible when invoked as bash, and script portability is generally excellent. The desktop alert functions have nothing to do with this discussion - the issue is the portability of modd.conf scripts. Desktop alerts are enabled through a flag on invocation, specifically because the modd.conf files need to remain portable. I am happy to (and plan to) add desktop alerts for Windows later on.

Again, I'm happy to have constructive discussions about other approaches. For instance, one possibility would be allowing users to change the invocation shell through a setting in the config file itself. If you're writing Windows cmd.exe-flavored scripts, you can then specify that and show users an error if the invocation shell can't be found.

[kardianos]

Sorry, I don't intend to come across poorly. I was just realizing that the tool you want might be slightly incompatible with the tool I want and trying to express that if that is the case, no issue.

I think the tool I would want to use wouldn't invoke bash or cmd at all, just execute the desired action. Then if you want other integration, such as desktop notification on exit != 0, build that in as well.

But that's a decent point, if the config file specified {raw,cmd,bash} you could then treat each config file similar as possible between different developers. +1 for that. (I might just say choose between raw (direct exec) and bash, and forget about cmd).

[cortesi]

I'd accept a patch for that. ;)

[cortesi]

Closing this ticket - the last release went out with a Windows build, and I think we've now covered off on all the points here.

[kardianos]

We need to document the shell variable and add an example or two. I might be able to push out a PR this weekend for that.