Skip to content
This repository was archived by the owner on Aug 30, 2024. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions internal/cmd/configssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ func binPath() (string, error) {
// Bash and OpenSSH for Windows (used by Powershell and VS Code) to function
// correctly. Check if the current executable is in $PATH, and warn the user
// if it isn't.
if runtime.GOOS == "windows" {
if runtime.GOOS == goosWindows {
binName := filepath.Base(exePath)

// We use safeexec instead of os/exec because os/exec returns paths in
Expand Down Expand Up @@ -268,7 +268,7 @@ func makeSSHConfig(binPath, workspaceName, privateKeyFilepath string, additional
fmt.Sprintf("IdentityFile=%q", privateKeyFilepath),
)

if runtime.GOOS == "linux" || runtime.GOOS == "darwin" {
if runtime.GOOS == goosLinux || runtime.GOOS == goosDarwin {
options = append(options,
"ControlMaster auto",
"ControlPath ~/.ssh/.connection-%r@%h:%p",
Expand Down
39 changes: 38 additions & 1 deletion internal/cmd/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ import (
"bufio"
"context"
"fmt"
"io/ioutil"
"net/url"
"os/exec"
"runtime"
"strings"

"github.com/pkg/browser"
Expand Down Expand Up @@ -66,7 +69,8 @@ func login(cmd *cobra.Command, workspaceURL *url.URL) error {
q.Add("show_token", "true")
authURL.RawQuery = q.Encode()

if err := browser.OpenURL(authURL.String()); err != nil {
if err := openURL(authURL.String()); err != nil {
clog.LogWarn(err.Error())
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the additional warning log 👍🏻

fmt.Printf("Open the following in your browser:\n\n\t%s\n\n", authURL.String())
} else {
fmt.Printf("Your browser has been opened to visit:\n\n\t%s\n\n", authURL.String())
Expand Down Expand Up @@ -113,3 +117,36 @@ func pingAPI(ctx context.Context, workspaceURL *url.URL, token string) error {
}
return nil
}

// isWSL determines if coder-cli is running within Windows Subsystem for Linux
func isWSL() (bool, error) {
if runtime.GOOS == goosDarwin || runtime.GOOS == goosWindows {
return false, nil
}
data, err := ioutil.ReadFile("/proc/version")
if err != nil {
return false, xerrors.Errorf("read /proc/version: %w", err)
}
return strings.Contains(strings.ToLower(string(data)), "microsoft"), nil
}

// openURL opens the provided URL via user's default browser
func openURL(url string) error {
var cmd string
var args []string

wsl, err := isWSL()
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is good, but I do wonder whether we could propose this upstream to pkg/browser

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the review. I agree, this work should ideally be part of upstream pkg/browser. But Thoughts on moving forward with the work for Coder until I have the patch ready for upstream?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I think it makes sense to make this change now for sure! No objection to it :)

Thanks for your contribution!

if err != nil {
return xerrors.Errorf("test running Windows Subsystem for Linux: %w", err)
}

if wsl {
cmd = "cmd.exe"
args = []string{"/c", "start"}
url = strings.ReplaceAll(url, "&", "^&")
args = append(args, url)
return exec.Command(cmd, args...).Start()
}

return browser.OpenURL(url)
}
3 changes: 2 additions & 1 deletion internal/cmd/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
const (
goosWindows = "windows"
goosLinux = "linux"
goosDarwin = "darwin"
apiPrivateVersion = "/api/private/version"
)

Expand Down Expand Up @@ -181,7 +182,7 @@ func (u *updater) Run(ctx context.Context, force bool, coderURLArg string, versi
// TODO: validate the checksum of the downloaded file. GitHub does not currently provide this information
// and we do not generate them yet.
var updatedBinaryName string
if u.osF() == "windows" {
if u.osF() == goosWindows {
updatedBinaryName = "coder.exe"
} else {
updatedBinaryName = "coder"
Expand Down