Adopt go 1.21 (Azure#2614)

This change moves us to use Go 1.21 for building `azd`. As part of go 1.21, the `maps`, `slices` and `slog` packages, previously in `golang.org/x/exp` have been moved into the standard libary. As part of this change, I've moved to use these versions of the packages instead of using the experimental versions. However, there were a few places where we were using `maps.Keys`, which was not added to the standard library. https://go-review.googlesource.com/c/go/+/513715 explains the rationale here, they may want `Keys` to return an iterator, and were not comfortable locking the name at this time. This functionality will be added to the `maps` package in a future release, it seems, likely under a name like `KeysSlice`. For now, I just kept the existing use of the the experimental package. We can update the import path and move to the stdlib version when it lands later. This change also updates `go.mod` to declare that we need at least go 1.21 to build `azd`. Developers which haven't upgraded will see build errors due to references of `maps`, `slices` and `log/slog` which do not exist in earlier versions of the standard libary. I've also enabled the loopvar experiment for our CI builds and tests. https://github.com/golang/go/wiki/LoopvarExperiment gives more information on what this does, but the long and short of it is that this behavior is likely to become the default in a future version of go, and the semantics of it more closely match what developers expect, so I would like us to just lock in the changes now (we are presently clean on this, so there are no changes, I just don't want us to introduce any new bad uses).
aaronpowell · Aug 21, 2023 · 7a7f4e7 · 7a7f4e7
1 parent cb50a37
commit 7a7f4e7
Show file tree

Hide file tree

Showing 27 changed files with 95 additions and 69 deletions.
diff --git a/.github/workflows/cli-ci.yml b/.github/workflows/cli-ci.yml
@@ -21,12 +21,12 @@ jobs:
     steps:
       - uses: actions/setup-go@v3
         with:
-          go-version: "^1.20.0"
+          go-version: "^1.21.0"
       - uses: actions/checkout@v3
       - name: golangci-lint
         uses: golangci/golangci-lint-action@v3
         with:
-          version: v1.51.2
+          version: v1.54.1
           args: -v --timeout 10m0s
           working-directory: cli/azd
 

diff --git a/cli/azd/ci-build.ps1 b/cli/azd/ci-build.ps1
@@ -132,43 +132,52 @@ function PrintFlags() {
     }
 }
 
-Write-Host "Running: go build ``"
-PrintFlags -flags $buildFlags
-go build @buildFlags
-
-if ($BuildRecordMode) {
-    $recordFlagPresent = $false
-    for ($i = 0; $i -lt $buildFlags.Length; $i++) {
-        if ($buildFlags[$i].StartsWith("-tags=")) {
-            $recordFlagPresent = $true
-            $buildFlags[$i] += ",record"
+$oldGOEXPERIMENT = $env:GOEXPERIMENT
+# Enable the loopvar experiment, which makes the loop variaible for go loops like `range` behave as most folks would expect.
+# the go team is exploring making this default in the future, and we'd like to opt into the behavior now.
+$env:GOEXPERIMENT="loopvar"
+
+try {
+    Write-Host "Running: go build ``"
+    PrintFlags -flags $buildFlags
+    go build @buildFlags
+
+    if ($BuildRecordMode) {
+        $recordFlagPresent = $false
+        for ($i = 0; $i -lt $buildFlags.Length; $i++) {
+            if ($buildFlags[$i].StartsWith("-tags=")) {
+                $recordFlagPresent = $true
+                $buildFlags[$i] += ",record"
+            }
+        }
+
+        if (-not $recordFlagPresent) {
+            $buildFlags[$i] += "-tags=record"
+        }
+
+        $outputFlag = "-o=azd-record"
+        if ($IsWindows) {
+            $outputFlag += ".exe"
         }
+        $buildFlags += $outputFlag
+
+        Write-Host "Running: go build (record) ``"
+        PrintFlags -flags $buildFlags
+        go build @buildFlags
     }
-
-    if (-not $recordFlagPresent) {
-        $buildFlags[$i] += "-tags=record"
+
+    if ($LASTEXITCODE) {
+        Write-Host "Error running go build"
+        exit $LASTEXITCODE
     }
-
-    $outputFlag = "-o=azd-record"
+    Write-Host "go build succeeded"
+
     if ($IsWindows) {
-        $outputFlag += ".exe"
+        Write-Host "Windows exe file version info"
+        $azdExe = Get-Item azd.exe
+        Write-Host "File Version: $($azdExe.VersionInfo.FileVersionRaw)"
+        Write-Host "Product Version: $($azdExe.VersionInfo.ProductVersionRaw)"
     }
-    $buildFlags += $outputFlag
-
-    Write-Host "Running: go build (record) ``"
-    PrintFlags -flags $buildFlags
-    go build @buildFlags
-}
-
-if ($LASTEXITCODE) {
-    Write-Host "Error running go build"
-    exit $LASTEXITCODE
-}
-Write-Host "go build succeeded"
-
-if ($IsWindows) {
-    Write-Host "Windows exe file version info"
-    $azdExe = Get-Item azd.exe
-    Write-Host "File Version: $($azdExe.VersionInfo.FileVersionRaw)"
-    Write-Host "Product Version: $($azdExe.VersionInfo.ProductVersionRaw)"
+} finally {
+    $env:GOEXPERIMENT = $oldGOEXPERIMENT    
 }
diff --git a/cli/azd/ci-test.ps1 b/cli/azd/ci-test.ps1
@@ -50,11 +50,22 @@ if ($ShortMode) {
 Write-Host "Running integration tests..."
 $intCoverDir = New-EmptyDirectory -Path $IntegrationTestCoverageDir
 
+$oldGOCOVERDIR = $env:GOCOVERDIR
+$oldGOEXPERIMENT = $env:GOEXPERIMENT
+
 # GOCOVERDIR enables any binaries (in this case, azd.exe) built with '-cover',
 # to write out coverage output to the specific directory.
 $env:GOCOVERDIR = $intCoverDir.FullName
+# Enable the loopvar experiment, which makes the loop variaible for go loops like `range` behave as most folks would expect.
+# the go team is exploring making this default in the future, and we'd like to opt into the behavior now.
+$env:GOEXPERIMENT="loopvar"
 
-& $gotestsum -- ./test/... -v -timeout $IntegrationTestTimeout
-if ($LASTEXITCODE) {
-    exit $LASTEXITCODE
-}
+try {
+    & $gotestsum -- ./test/... -v -timeout $IntegrationTestTimeout
+    if ($LASTEXITCODE) {
+        exit $LASTEXITCODE
+    }    
+} finally {
+    $env:GOCOVERDIR = $oldGOCOVERDIR
+    $env:GOEXPERIMENT = $oldGOEXPERIMENT
+}
diff --git a/cli/azd/cmd/cmd_help.go b/cli/azd/cmd/cmd_help.go
@@ -5,12 +5,12 @@ package cmd
 
 import (
 	"fmt"
+	"slices"
 	"strings"
 
 	"github.com/azure/azure-dev/cli/azd/pkg/output"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
-	"golang.org/x/exp/slices"
 )
 
 const (

diff --git a/cli/azd/cmd/cobra_builder.go b/cli/azd/cmd/cobra_builder.go
@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"log"
+	"slices"
 	"strconv"
 	"strings"
 
@@ -19,7 +20,6 @@ import (
 	"github.com/azure/azure-dev/cli/azd/pkg/tools"
 	"github.com/azure/azure-dev/cli/azd/pkg/tools/azcli"
 	"github.com/spf13/cobra"
-	"golang.org/x/exp/slices"
 )
 
 const cDocsFlagName = "docs"

diff --git a/cli/azd/internal/repository/initializer_test.go b/cli/azd/internal/repository/initializer_test.go
@@ -7,6 +7,7 @@ import (
 	"os"
 	"path/filepath"
 	"runtime"
+	"slices"
 	"strings"
 	"testing"
 
@@ -21,7 +22,6 @@ import (
 	"github.com/azure/azure-dev/cli/azd/test/mocks/mockinput"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"golang.org/x/exp/slices"
 )
 
 func Test_Initializer_Initialize(t *testing.T) {

diff --git a/cli/azd/internal/telemetry/storage_test.go b/cli/azd/internal/telemetry/storage_test.go
@@ -5,14 +5,14 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"slices"
 	"testing"
 	"time"
 
 	"github.com/azure/azure-dev/cli/azd/pkg/osutil"
 	"github.com/benbjohnson/clock"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"golang.org/x/exp/slices"
 )
 
 // The tests in this file intentionally interacts with the filesystem (important implementation detail).

diff --git a/cli/azd/internal/telemetry/uploader_test.go b/cli/azd/internal/telemetry/uploader_test.go
@@ -4,14 +4,14 @@ import (
 	"context"
 	"fmt"
 	"math/rand"
+	"slices"
 	"strconv"
 	"testing"
 	"time"
 
 	appinsightsexporter "github.com/azure/azure-dev/cli/azd/internal/telemetry/appinsights-exporter"
 	"github.com/benbjohnson/clock"
 	"github.com/stretchr/testify/assert"
-	"golang.org/x/exp/slices"
 )
 
 type InMemoryItem struct {

diff --git a/cli/azd/internal/tracing/baggage/baggage.go b/cli/azd/internal/tracing/baggage/baggage.go
@@ -4,8 +4,9 @@
 package baggage
 
 import (
-	"go.opentelemetry.io/otel/attribute"
 	"golang.org/x/exp/maps"
+
+	"go.opentelemetry.io/otel/attribute"
 )
 
 // An immutable object safe for concurrent use.

diff --git a/cli/azd/pkg/account/manager.go b/cli/azd/pkg/account/manager.go
@@ -6,9 +6,9 @@ import (
 	"fmt"
 	"log"
 	"os"
+	"slices"
 
 	"github.com/azure/azure-dev/cli/azd/pkg/config"
-	"golang.org/x/exp/slices"
 )
 
 // JSON document path locations for default subscription & location

diff --git a/cli/azd/pkg/account/manager_test.go b/cli/azd/pkg/account/manager_test.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"net/http"
+	"slices"
 	"testing"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/resources/armsubscriptions"
@@ -15,7 +16,6 @@ import (
 	"github.com/azure/azure-dev/cli/azd/test/mocks/mockhttp"
 	"github.com/azure/azure-dev/cli/azd/test/mocks/mockinput"
 	"github.com/stretchr/testify/require"
-	"golang.org/x/exp/slices"
 )
 
 func Test_GetAccountDefaults(t *testing.T) {

diff --git a/cli/azd/pkg/auth/errors.go b/cli/azd/pkg/auth/errors.go
@@ -8,9 +8,9 @@ import (
 	"io"
 	"log"
 	"net/http"
+	"slices"
 
 	msal "github.com/AzureAD/microsoft-authentication-library-for-go/apps/errors"
-	"golang.org/x/exp/slices"
 )
 
 const cLoginCmd = "azd auth login"

diff --git a/cli/azd/pkg/azureutil/location.go b/cli/azd/pkg/azureutil/location.go
@@ -7,12 +7,12 @@ import (
 	"context"
 	"fmt"
 	"os"
+	"slices"
 	"strings"
 
 	"github.com/azure/azure-dev/cli/azd/pkg/account"
 	"github.com/azure/azure-dev/cli/azd/pkg/environment"
 	"github.com/azure/azure-dev/cli/azd/pkg/input"
-	"golang.org/x/exp/slices"
 )
 
 // PromptLocation asks the user to select a location from a list of supported azure locations for a given subscription.
@@ -48,9 +48,9 @@ func PromptLocationWithFilter(
 		}
 	}
 
-	slices.SortFunc(locations, func(a, b account.Location) bool {
+	slices.SortFunc(locations, func(a, b account.Location) int {
 		return strings.Compare(
-			strings.ToLower(a.RegionalDisplayName), strings.ToLower(b.RegionalDisplayName)) < 0
+			strings.ToLower(a.RegionalDisplayName), strings.ToLower(b.RegionalDisplayName))
 	})
 
 	// Allow the environment variable `AZURE_LOCATION` to control the default value for the location

diff --git a/cli/azd/pkg/environment/environment.go b/cli/azd/pkg/environment/environment.go
@@ -11,13 +11,14 @@ import (
 	"regexp"
 	"strings"
 
+	"maps"
+
 	"github.com/azure/azure-dev/cli/azd/internal/tracing"
 	"github.com/azure/azure-dev/cli/azd/internal/tracing/fields"
 	"github.com/azure/azure-dev/cli/azd/pkg/config"
 	"github.com/azure/azure-dev/cli/azd/pkg/environment/azdcontext"
 	"github.com/azure/azure-dev/cli/azd/pkg/osutil"
 	"github.com/joho/godotenv"
-	"golang.org/x/exp/maps"
 )
 
 // EnvNameEnvVarName is the name of the key used to store the envname property in the environment.

diff --git a/cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go b/cli/azd/pkg/infra/provisioning/bicep/bicep_provider.go
@@ -12,6 +12,7 @@ import (
 	"math"
 	"os"
 	"path/filepath"
+	"slices"
 	"strconv"
 	"strings"
 	"time"
@@ -39,7 +40,6 @@ import (
 	"github.com/benbjohnson/clock"
 	"github.com/drone/envsubst"
 	"golang.org/x/exp/maps"
-	"golang.org/x/exp/slices"
 )
 
 const DefaultModule = "main"
@@ -739,8 +739,8 @@ func (p *BicepProvider) findCompletedDeployments(
 		return nil, err
 	}
 
-	slices.SortFunc(deployments, func(x, y *armresources.DeploymentExtended) bool {
-		return x.Properties.Timestamp.After(*y.Properties.Timestamp)
+	slices.SortFunc(deployments, func(x, y *armresources.DeploymentExtended) int {
+		return x.Properties.Timestamp.Compare(*y.Properties.Timestamp)
 	})
 
 	// If hint is not provided, use the environment name as the hint

diff --git a/cli/azd/pkg/infra/provisioning/bicep/prompt.go b/cli/azd/pkg/infra/provisioning/bicep/prompt.go
@@ -4,13 +4,13 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"slices"
 	"strconv"
 
 	"github.com/azure/azure-dev/cli/azd/pkg/account"
 	"github.com/azure/azure-dev/cli/azd/pkg/azure"
 	"github.com/azure/azure-dev/cli/azd/pkg/input"
 	"github.com/azure/azure-dev/cli/azd/pkg/output"
-	"golang.org/x/exp/slices"
 
 	. "github.com/azure/azure-dev/cli/azd/pkg/infra/provisioning"
 )

diff --git a/cli/azd/pkg/pipeline/github_provider.go b/cli/azd/pkg/pipeline/github_provider.go
@@ -13,6 +13,7 @@ import (
 	"net/url"
 	"path/filepath"
 	"regexp"
+	"slices"
 	"strings"
 
 	"github.com/Azure/azure-sdk-for-go/sdk/azcore"
@@ -31,7 +32,6 @@ import (
 	"github.com/azure/azure-dev/cli/azd/pkg/tools/azcli"
 	"github.com/azure/azure-dev/cli/azd/pkg/tools/git"
 	"github.com/azure/azure-dev/cli/azd/pkg/tools/github"
-	"golang.org/x/exp/slices"
 )
 
 // GitHubScmProvider implements ScmProvider using GitHub as the provider

diff --git a/cli/azd/pkg/pipeline/pipeline_manager.go b/cli/azd/pkg/pipeline/pipeline_manager.go
@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"log"
 	"path/filepath"
+	"slices"
 	"strings"
 	"time"
 
@@ -25,7 +26,6 @@ import (
 	"github.com/azure/azure-dev/cli/azd/pkg/tools/azcli"
 	"github.com/azure/azure-dev/cli/azd/pkg/tools/git"
 	"github.com/sethvargo/go-retry"
-	"golang.org/x/exp/slices"
 )
 
 type PipelineAuthType string

diff --git a/cli/azd/pkg/project/project.go b/cli/azd/pkg/project/project.go
@@ -7,6 +7,7 @@ import (
 	"log"
 	"os"
 	"path/filepath"
+	"slices"
 	"strings"
 
 	"github.com/azure/azure-dev/cli/azd/internal"
@@ -16,7 +17,6 @@ import (
 	"github.com/azure/azure-dev/cli/azd/pkg/infra/provisioning"
 	"github.com/azure/azure-dev/cli/azd/pkg/osutil"
 	"github.com/blang/semver/v4"
-	"golang.org/x/exp/slices"
 	"gopkg.in/yaml.v3"
 )