Address PR comments

coder · johnstcn · Jun 13, 2022 · Jun 9, 2022 · Jun 10, 2022 · Jun 10, 2022
commit 25ef43750b336b0f41765a7c5f98eb3a73361a84
diff --git a/cli/autostart.go b/cli/autostart.go
@@ -15,7 +15,7 @@ import (
 )
 
 const autostartDescriptionLong = `To have your workspace build automatically at a regular time you can enable autostart.
-When enabling autostart, enter a schedule in the format: start-time [day-of-week] [location].
+When enabling autostart, enter a schedule in the format: <start-time> [day-of-week] [location].
   * Start-time (required) is accepted either in 12-hour (hh:mm{am|pm}) format, or 24-hour format hh:mm.
   * Day-of-week (optional) allows specifying in the cron format, e.g. 1,3,5 or Mon-Fri.
     Aliases such as @daily are not supported.
@@ -225,12 +225,13 @@ func parseTime(s string) (time.Time, error) {
 	// Try a number of possible layouts.
 	for _, layout := range []string{
 		time.Kitchen, // 03:04PM
+		"03:04pm",
+		"3:04PM",
+		"3:04pm",
 		"15:04",
 		"1504",
-		"3pm",
-		"3PM",
-		"3:04pm",
-		"3:04PM",
+		"03PM",
+		"03pm",
 	} {
 		t, err := time.Parse(layout, s)
 		if err == nil {

diff --git a/cli/autostart_internal_test.go b/cli/autostart_internal_test.go
@@ -57,6 +57,18 @@ func TestParseCLISchedule(t *testing.T) {
 			expectedSchedule: "CRON_TZ=America/Chicago 0 9 * * *",
 			tzEnv:            "UTC",
 		},
+		{
+			name:             "LazyTime",
+			input:            []string{"9am", "America/Chicago"},
+			expectedSchedule: "CRON_TZ=America/Chicago 0 9 * * *",
+			tzEnv:            "UTC",
+		},
+		{
+			name:             "ZeroPrefixedLazyTime",
+			input:            []string{"09am", "America/Chicago"},
+			expectedSchedule: "CRON_TZ=America/Chicago 0 9 * * *",
+			tzEnv:            "UTC",
+		},
 		{
 			name:          "InvalidTime",
 			input:         []string{"nine"},

diff --git a/coderd/util/tz/tz.go b/coderd/util/tz/tz.go
@@ -0,0 +1,30 @@
+// Package tz includes utilities for cross-platform timezone/location detection.
+package tz
+
+import (
+	"os"
+	"time"
+
+	"golang.org/x/xerrors"
+)
+
+var errNoEnvSet = xerrors.New("no env set")
+
+func locationFromEnv() (*time.Location, error) {
+	tzEnv, found := os.LookupEnv("TZ")
+	if !found {
+		return nil, errNoEnvSet
+	}
+
+	// TZ set but empty means UTC.
+	if tzEnv == "" {
+		return time.UTC, nil
+	}
+
+	loc, err := time.LoadLocation(tzEnv)
+	if err != nil {
+		return nil, xerrors.Errorf("load location from TZ env: %w", err)
+	}
+
+	return loc, nil
+}
diff --git a/coderd/util/tz/tz_darwin.go b/coderd/util/tz/tz_darwin.go
@@ -22,16 +22,13 @@ const zoneInfoPath = "/var/db/timezone/zoneinfo/"
 // is used instead to get the current time location in IANA format.
 // Reference: https://superuser.com/a/1584968
 func TimezoneIANA() (*time.Location, error) {
-	if tzEnv, found := os.LookupEnv("TZ"); found {
-		if tzEnv == "" {
-			return time.UTC, nil
-		}
-		loc, err := time.LoadLocation(tzEnv)
-		if err != nil {
-			return nil, xerrors.Errorf("load location from TZ env: %w", err)
-		}
+	loc, err := locationFromEnv()
+	if err == nil {
 		return loc, nil
 	}
+	if !xerrors.Is(err, errNoEnvSet) {
+		return nil, xerrors.Errorf("lookup timezone from env: %w", err)
+	}
 
 	lp, err := filepath.EvalSymlinks(etcLocaltime)
 	if err != nil {
@@ -46,7 +43,7 @@ func TimezoneIANA() (*time.Location, error) {
 
 	stripped := strings.Replace(lp, realZoneInfoPath, "", -1)
 	stripped = strings.TrimPrefix(stripped, string(filepath.Separator))
-	loc, err := time.LoadLocation(stripped)
+	loc, err = time.LoadLocation(stripped)
 	if err != nil {
 		return nil, xerrors.Errorf("invalid location %q guessed from %s: %w", stripped, lp, err)
 	}

diff --git a/coderd/util/tz/tz_linux.go b/coderd/util/tz/tz_linux.go
@@ -3,7 +3,6 @@
 package tz
 
 import (
-	"os"
 	"path/filepath"
 	"strings"
 	"time"
@@ -22,17 +21,13 @@ const zoneInfoPath = "/usr/share/zoneinfo"
 // is used instead to get the current time location in IANA format.
 // Reference: https://superuser.com/a/1584968
 func TimezoneIANA() (*time.Location, error) {
-	if tzEnv, found := os.LookupEnv("TZ"); found {
-		// TZ set but empty means UTC.
-		if tzEnv == "" {
-			return time.UTC, nil
-		}
-		loc, err := time.LoadLocation(tzEnv)
-		if err != nil {
-			return nil, xerrors.Errorf("load location from TZ env: %w", err)
-		}
+	loc, err := locationFromEnv()
+	if err == nil {
 		return loc, nil
 	}
+	if !xerrors.Is(err, errNoEnvSet) {
+		return nil, xerrors.Errorf("lookup timezone from env: %w", err)
+	}
 
 	lp, err := filepath.EvalSymlinks(etcLocaltime)
 	if err != nil {
@@ -41,7 +36,7 @@ func TimezoneIANA() (*time.Location, error) {
 
 	stripped := strings.Replace(lp, zoneInfoPath, "", -1)
 	stripped = strings.TrimPrefix(stripped, string(filepath.Separator))
-	loc, err := time.LoadLocation(stripped)
+	loc, err = time.LoadLocation(stripped)
 	if err != nil {
 		return nil, xerrors.Errorf("invalid location %q guessed from %s: %w", stripped, lp, err)
 	}

diff --git a/coderd/util/tz/tz_windows.go b/coderd/util/tz/tz_windows.go
@@ -24,32 +24,18 @@ const cmdTimezone = "[Windows.Globalization.Calendar,Windows.Globalization,Conte
 // is used instead to get the current time location in IANA format.
 // Reference: https://superuser.com/a/1584968
 func TimezoneIANA() (*time.Location, error) {
-	if tzEnv, found := os.LookupEnv("TZ"); found {
-		if tzEnv == "" {
-			return time.UTC, nil
-		}
-		loc, err := time.LoadLocation(tzEnv)
-		if err != nil {
-			return nil, xerrors.Errorf("load location from TZ env: %w", err)
-		}
+	loc, err := locationFromEnv()
+	if err == nil {
 		return loc, nil
 	}
+	if !xerrors.Is(err, errNoEnvSet) {
+		return nil, xerrors.Errorf("lookup timezone from env: %w", err)
+	}
 
 	// https://superuser.com/a/1584968
 	cmd := exec.Command("powershell.exe", "-NoLogo", "-NoProfile", "-NonInteractive")
-	stdin, err := cmd.StdinPipe()
-	if err != nil {
-		return nil, xerrors.Errorf("run powershell: %w", err)
-	}
-
-	done := make(chan struct{})
-	go func() {
-		defer stdin.Close()
-		defer close(done)
-		_, _ = fmt.Fprintln(stdin, cmdTimezone)
-	}()
-
-	<-done
+	// Powershell echoes its stdin so write a newline
+	cmd.Stdin = strings.NewReader(cmdTimezone + "\n")
 
 	outBytes, err := cmd.CombinedOutput()
 	if err != nil {
@@ -62,7 +48,7 @@ func TimezoneIANA() (*time.Location, error) {
 	}
 	// What we want is the second line of output
 	locStr := strings.TrimSpace(outLines[1])
-	loc, err := time.LoadLocation(locStr)
+	loc, err = time.LoadLocation(locStr)
 	if err != nil {
 		return nil, xerrors.Errorf("invalid location %q from powershell: %w", locStr, err)
 	}