Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

initial implementation of macOS crypto backend #1453

Open
wants to merge 8 commits into
base: microsoft/main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
17 changes: 17 additions & 0 deletions eng/_util/buildutil/buildutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ func AppendExperimentEnv(experiment string) {
if strings.Contains(experiment, "opensslcrypto") ||
strings.Contains(experiment, "cngcrypto") ||
strings.Contains(experiment, "boringcrypto") ||
strings.Contains(experiment, "darwincrypto") ||
strings.Contains(experiment, "systemcrypto") {

experiment += ",allowcryptofallback"
Expand All @@ -103,3 +104,19 @@ func AppendExperimentEnv(experiment string) {
panic(err)
}
}

// UnassignGOROOT unsets the GOROOT env var if it is set.
//
// Setting GOROOT explicitly in the environment has not been necessary since Go
// 1.9 (https://go.dev/doc/go1.9#goroot), but a dev or build machine may still
// have it set. It interferes with attempts to run the built Go (such as when
// building the race runtime), so remove the explicit GOROOT if set.
func UnassignGOROOT() error {
if explicitRoot, ok := os.LookupEnv("GOROOT"); ok {
fmt.Printf("---- Removing explicit GOROOT from environment: %v\n", explicitRoot)
if err := os.Unsetenv("GOROOT"); err != nil {
return err
}
}
return nil
}
11 changes: 2 additions & 9 deletions eng/_util/cmd/build/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,15 +138,8 @@ func build(o *options) error {
}
fmt.Printf("---- Target platform: %v_%v\n", targetOS, targetArch)

// Setting GOROOT explicitly in the environment has not been necessary since Go 1.9
// (https://go.dev/doc/go1.9#goroot), but a dev or build machine may still have it set. It
// interferes with attempts to run the built Go (such as when building the race runtime), so
// remove the explicit GOROOT if set.
if explicitRoot, ok := os.LookupEnv("GOROOT"); ok {
fmt.Printf("---- Removing explicit GOROOT from environment: %v\n", explicitRoot)
if err := os.Unsetenv("GOROOT"); err != nil {
return err
}
if err := buildutil.UnassignGOROOT(); err != nil {
return err
}

// The upstream build scripts in {repo-root}/src require your working directory to be src, or
Expand Down
4 changes: 4 additions & 0 deletions eng/_util/cmd/run-builder/run-builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,10 @@ func main() {
env("GO_TEST_TIMEOUT_SCALE", strconv.Itoa(timeoutScale))
}

if err := buildutil.UnassignGOROOT(); err != nil {
log.Fatal(err)
}

buildCmdline := []string{"pwsh", "eng/run.ps1", "build"}

// run.ps1 compiles Go code, so we can't use the experiment yet. We must pass the experiment
Expand Down
7 changes: 7 additions & 0 deletions eng/pipeline/stages/go-builder-matrix-stages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,13 @@ stages:
- { os: linux, arch: arm64, config: buildandpack }
- ${{ if parameters.innerloop }}:
- { os: darwin, arch: amd64, config: devscript }
- { os: darwin, arch: amd64, config: test }
- { experiment: darwincrypto, os: darwin, arch: amd64, config: test }
- { experiment: darwincrypto, os: darwin, arch: amd64, config: test, fips: true }
- { os: darwin, arch: arm64, config: devscript }
- { os: darwin, arch: arm64, config: test }
- { experiment: darwincrypto, os: darwin, arch: arm64, config: test }
- { experiment: darwincrypto, os: darwin, arch: arm64, config: test, fips: true }
- { os: linux, arch: amd64, config: devscript }
- { os: linux, arch: amd64, config: test }
- { os: linux, arch: amd64, config: test, distro: ubuntu }
Expand Down
8 changes: 6 additions & 2 deletions eng/pipeline/stages/pool-2.yml
Original file line number Diff line number Diff line change
Expand Up @@ -53,5 +53,9 @@ stages:

${{ elseif eq(parameters.os, 'darwin') }}:
# https://learn.microsoft.com/en-us/azure/devops/pipelines/agents/hosted?view=azure-devops&tabs=yaml#software
vmImage: 'macos-14'
os: macOs
${{ if eq(parameters.hostArch, 'amd64') }}:
vmImage: 'macos-14'
os: macOS
${{ else }}:
vmImage: 'macos-latest-internal'
os: macOS
64 changes: 44 additions & 20 deletions patches/0001-Add-systemcrypto-GOEXPERIMENT.patch
Original file line number Diff line number Diff line change
Expand Up @@ -11,18 +11,18 @@ information about the behavior.
Includes new tests in "build_test.go" and "buildbackend_test.go" to help
maintain this feature. For more information, see the test files.
---
src/cmd/go/internal/modindex/build.go | 54 ++++++++++++++
src/cmd/go/internal/modindex/build_test.go | 73 +++++++++++++++++++
src/go/build/build.go | 54 ++++++++++++++
src/go/build/buildbackend_test.go | 66 +++++++++++++++++
src/cmd/go/internal/modindex/build.go | 57 +++++++++++++
src/cmd/go/internal/modindex/build_test.go | 73 ++++++++++++++++
src/go/build/build.go | 57 +++++++++++++
src/go/build/buildbackend_test.go | 84 +++++++++++++++++++
.../testdata/backendtags_openssl/main.go | 3 +
.../testdata/backendtags_openssl/openssl.go | 3 +
.../build/testdata/backendtags_system/main.go | 3 +
.../backendtags_system/systemcrypto.go | 3 +
.../goexperiment/exp_systemcrypto_off.go | 9 +++
.../goexperiment/exp_systemcrypto_on.go | 9 +++
.../goexperiment/exp_systemcrypto_off.go | 9 ++
.../goexperiment/exp_systemcrypto_on.go | 9 ++
src/internal/goexperiment/flags.go | 15 ++++
11 files changed, 292 insertions(+)
11 files changed, 316 insertions(+)
create mode 100644 src/cmd/go/internal/modindex/build_test.go
create mode 100644 src/go/build/buildbackend_test.go
create mode 100644 src/go/build/testdata/backendtags_openssl/main.go
Expand All @@ -33,16 +33,17 @@ maintain this feature. For more information, see the test files.
create mode 100644 src/internal/goexperiment/exp_systemcrypto_on.go

diff --git a/src/cmd/go/internal/modindex/build.go b/src/cmd/go/internal/modindex/build.go
index b57f2f6368f0fe..9ddde1ce9a2286 100644
index b4dacb0f523a8d..4315c288d10cb3 100644
--- a/src/cmd/go/internal/modindex/build.go
+++ b/src/cmd/go/internal/modindex/build.go
@@ -880,13 +880,67 @@ func (ctxt *Context) matchTag(name string, allTags map[string]bool) bool {
@@ -886,13 +886,70 @@ func (ctxt *Context) matchTag(name string, allTags map[string]bool) bool {
name = "goexperiment.boringcrypto" // boringcrypto is an old name for goexperiment.boringcrypto
}

+ const system = "goexperiment.systemcrypto"
+ const openssl = "goexperiment.opensslcrypto"
+ const cng = "goexperiment.cngcrypto"
+ const darwin = "goexperiment.darwincrypto"
+ const boring = "goexperiment.boringcrypto"
+ // Implement the SystemCrypto GOEXPERIMENT logic. This is done here rather
+ // than during GOEXPERIMENT parsing so "-tags goexperiment.systemcrypto"
Expand All @@ -63,11 +64,12 @@ index b57f2f6368f0fe..9ddde1ce9a2286 100644
+ satisfiedByAnyBackend := name == system
+ satisfiedBySystemCrypto :=
+ (ctxt.GOOS == "linux" && name == openssl) ||
+ (ctxt.GOOS == "windows" && name == cng)
+ (ctxt.GOOS == "windows" && name == cng) ||
+ (ctxt.GOOS == "darwin" && name == darwin)
+ satisfiedBy := func(tag string) bool {
+ if satisfiedByAnyBackend {
+ switch tag {
+ case openssl, cng, boring:
+ case openssl, cng, darwin, boring:
+ return true
+ }
+ }
Expand All @@ -81,6 +83,7 @@ index b57f2f6368f0fe..9ddde1ce9a2286 100644
+ if satisfiedByAnyBackend {
+ allTags[openssl] = true
+ allTags[cng] = true
+ allTags[darwin] = true
+ allTags[boring] = true
+ }
+ if satisfiedBySystemCrypto {
Expand Down Expand Up @@ -184,16 +187,17 @@ index 00000000000000..1756c5d027fee0
+ }
+}
diff --git a/src/go/build/build.go b/src/go/build/build.go
index dd6cdc903a21a8..48adcfed5cf3cb 100644
index 9ffffda08a99b1..78fd536fa6a6d1 100644
--- a/src/go/build/build.go
+++ b/src/go/build/build.go
@@ -1947,13 +1947,67 @@ func (ctxt *Context) matchTag(name string, allTags map[string]bool) bool {
@@ -1984,13 +1984,70 @@ func (ctxt *Context) matchTag(name string, allTags map[string]bool) bool {
name = "goexperiment.boringcrypto" // boringcrypto is an old name for goexperiment.boringcrypto
}

+ const system = "goexperiment.systemcrypto"
+ const openssl = "goexperiment.opensslcrypto"
+ const cng = "goexperiment.cngcrypto"
+ const darwin = "goexperiment.darwincrypto"
+ const boring = "goexperiment.boringcrypto"
+ // Implement the SystemCrypto GOEXPERIMENT logic. This is done here rather
+ // than during GOEXPERIMENT parsing so "-tags goexperiment.systemcrypto"
Expand All @@ -214,11 +218,12 @@ index dd6cdc903a21a8..48adcfed5cf3cb 100644
+ satisfiedByAnyBackend := name == system
+ satisfiedBySystemCrypto :=
+ (ctxt.GOOS == "linux" && name == openssl) ||
+ (ctxt.GOOS == "windows" && name == cng)
+ (ctxt.GOOS == "windows" && name == cng) ||
+ (ctxt.GOOS == "darwin" && name == darwin)
+ satisfiedBy := func(tag string) bool {
+ if satisfiedByAnyBackend {
+ switch tag {
+ case openssl, cng, boring:
+ case openssl, cng, darwin, boring:
+ return true
+ }
+ }
Expand All @@ -232,6 +237,7 @@ index dd6cdc903a21a8..48adcfed5cf3cb 100644
+ if satisfiedByAnyBackend {
+ allTags[openssl] = true
+ allTags[cng] = true
+ allTags[darwin] = true
+ allTags[boring] = true
+ }
+ if satisfiedBySystemCrypto {
Expand All @@ -257,10 +263,10 @@ index dd6cdc903a21a8..48adcfed5cf3cb 100644
}
diff --git a/src/go/build/buildbackend_test.go b/src/go/build/buildbackend_test.go
new file mode 100644
index 00000000000000..a22abbb42e37c0
index 00000000000000..aa3c5f1007ed79
--- /dev/null
+++ b/src/go/build/buildbackend_test.go
@@ -0,0 +1,66 @@
@@ -0,0 +1,84 @@
+// Copyright 2023 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
Expand Down Expand Up @@ -318,14 +324,32 @@ index 00000000000000..a22abbb42e37c0
+ if err != nil {
+ t.Fatal(err)
+ }
+ want = []string{"goexperiment.boringcrypto", "goexperiment.cngcrypto", "goexperiment.opensslcrypto", "goexperiment.systemcrypto"}
+ want = []string{"goexperiment.boringcrypto", "goexperiment.cngcrypto", "goexperiment.darwincrypto", "goexperiment.opensslcrypto", "goexperiment.systemcrypto"}
+ if !reflect.DeepEqual(p.AllTags, want) {
+ t.Errorf("AllTags = %v, want %v", p.AllTags, want)
+ }
+ wantFiles = []string{"main.go", "systemcrypto.go"}
+ if !reflect.DeepEqual(p.GoFiles, wantFiles) {
+ t.Errorf("GoFiles = %v, want %v", p.GoFiles, wantFiles)
+ }
+
+ ctxt.GOARCH = "amd64"
+ ctxt.GOOS = "darwin"
+ ctxt.BuildTags = []string{"goexperiment.darwincrypto"}
+ p, err = ctxt.ImportDir("testdata/backendtags_openssl", 0)
+ if err != nil {
+ t.Fatal(err)
+ }
+ // Given the current GOOS (darwin), systemcrypto would not affect the
+ // decision, so we don't want it to be included in AllTags.
+ want = []string{"goexperiment.opensslcrypto"}
+ if !reflect.DeepEqual(p.AllTags, want) {
+ t.Errorf("AllTags = %v, want %v", p.AllTags, want)
+ }
+ wantFiles = []string{"main.go"}
+ if !reflect.DeepEqual(p.GoFiles, wantFiles) {
+ t.Errorf("GoFiles = %v, want %v", p.GoFiles, wantFiles)
+ }
+}
diff --git a/src/go/build/testdata/backendtags_openssl/main.go b/src/go/build/testdata/backendtags_openssl/main.go
new file mode 100644
Expand Down Expand Up @@ -394,14 +418,14 @@ index 00000000000000..9c5b0bbc7b99dc
+const SystemCrypto = true
+const SystemCryptoInt = 1
diff --git a/src/internal/goexperiment/flags.go b/src/internal/goexperiment/flags.go
index ae3cbaf89fa5dd..de79140b2d4780 100644
index 31b3d0315b64f8..de1dfa6e567a71 100644
--- a/src/internal/goexperiment/flags.go
+++ b/src/internal/goexperiment/flags.go
@@ -60,6 +60,21 @@ type Flags struct {
StaticLockRanking bool
BoringCrypto bool

+ // SystemCrypto enables the OpenSSL or CNG crypto experiment depending on
+ // SystemCrypto enables the OpenSSL, CNG or Darwin crypto experiment depending on
+ // which one is appropriate on the target GOOS.
+ //
+ // If SystemCrypto is enabled but no crypto experiment is appropriate on the
Expand Down
Loading
Loading