From 87a6513725256c50895a1cc40aefd2cbb78a3a75 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 14 Apr 2026 00:04:22 +0200 Subject: [PATCH 1/2] Replace sync.Once with sync.OnceFunc/OnceValue/OnceValues Migrate all sync.Once usage to the typed helpers added in Go 1.21. Add a forbidigo lint rule to prevent sync.Once from being reintroduced. Co-authored-by: Isaac --- .golangci.yaml | 2 ++ bundle/bundle.go | 39 ++++++++++---------- bundle/direct/dresources/config.go | 58 ++++++++++++------------------ internal/build/info.go | 13 +++---- internal/build/variables.go | 6 ++-- libs/apps/logstream/streamer.go | 11 +++--- libs/apps/vite/bridge.go | 34 ++++++++++-------- libs/cmdio/spinner.go | 27 +++++++------- 8 files changed, 89 insertions(+), 101 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index e64590f365..2ab0ac30fc 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -46,6 +46,8 @@ linters: msg: Use env.UserHomeDir(ctx) from libs/env instead. - pattern: 'os\.Getenv' msg: Use env.Get(ctx) from the libs/env package instead of os.Getenv. + - pattern: 'sync\.Once\b($|[^FV])' + msg: Use sync.OnceFunc, sync.OnceValue, or sync.OnceValues instead. analyze-types: true copyloopvar: check-alias: true diff --git a/bundle/bundle.go b/bundle/bundle.go index 97824eb839..93fba5d3d8 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -8,7 +8,6 @@ package bundle import ( "context" - "errors" "fmt" "os" "path/filepath" @@ -123,9 +122,7 @@ type Bundle struct { // Store a pointer to the workspace client. // It can be initialized on demand after loading the configuration. - clientOnce sync.Once - client *databricks.WorkspaceClient - clientErr error + getClient func() (*databricks.WorkspaceClient, error) // Files that are synced to the workspace.file_path Files []fileset.File @@ -225,16 +222,21 @@ func TryLoad(ctx context.Context) *Bundle { return b } -func (b *Bundle) WorkspaceClientE() (*databricks.WorkspaceClient, error) { - b.clientOnce.Do(func() { - var err error - b.client, err = b.Config.Workspace.Client() +func (b *Bundle) initClientOnce() { + b.getClient = sync.OnceValues(func() (*databricks.WorkspaceClient, error) { + w, err := b.Config.Workspace.Client() if err != nil { - b.clientErr = fmt.Errorf("cannot resolve bundle auth configuration: %w", err) + return nil, fmt.Errorf("cannot resolve bundle auth configuration: %w", err) } + return w, nil }) +} - return b.client, b.clientErr +func (b *Bundle) WorkspaceClientE() (*databricks.WorkspaceClient, error) { + if b.getClient == nil { + b.initClientOnce() + } + return b.getClient() } func (b *Bundle) WorkspaceClient() *databricks.WorkspaceClient { @@ -249,16 +251,15 @@ func (b *Bundle) WorkspaceClient() *databricks.WorkspaceClient { // SetWorkpaceClient sets the workspace client for this bundle. // This is used to inject a mock client for testing. func (b *Bundle) SetWorkpaceClient(w *databricks.WorkspaceClient) { - b.clientOnce.Do(func() {}) - b.client = w + b.getClient = func() (*databricks.WorkspaceClient, error) { + return w, nil + } } // ClearWorkspaceClient resets the workspace client cache, allowing // WorkspaceClientE() to attempt client creation again on the next call. func (b *Bundle) ClearWorkspaceClient() { - b.clientOnce = sync.Once{} - b.client = nil - b.clientErr = nil + b.initClientOnce() } // LocalStateDir returns directory to use for temporary files for this bundle without creating @@ -347,12 +348,12 @@ func (b *Bundle) GetSyncIncludePatterns(ctx context.Context) ([]string, error) { // This map can be used to configure authentication for tools that // we call into from this bundle context. func (b *Bundle) AuthEnv() (map[string]string, error) { - if b.client == nil { - return nil, errors.New("workspace client not initialized yet") + w, err := b.WorkspaceClientE() + if err != nil { + return nil, err } - cfg := b.client.Config - return auth.Env(cfg), nil + return auth.Env(w.Config), nil } // StateFilenameDirect returns (relative remote path, relative local path) for direct engine resource state diff --git a/bundle/direct/dresources/config.go b/bundle/direct/dresources/config.go index 0b71f51382..70f6dbb1d8 100644 --- a/bundle/direct/dresources/config.go +++ b/bundle/direct/dresources/config.go @@ -75,48 +75,36 @@ var resourcesYAML []byte //go:embed resources.generated.yml var resourcesGeneratedYAML []byte -var ( - configOnce sync.Once - globalConfig *Config - generatedConfigOnce sync.Once - generatedConfig *Config - empty = ResourceLifecycleConfig{ - IgnoreRemoteChanges: nil, - IgnoreLocalChanges: nil, - RecreateOnChanges: nil, - UpdateIDOnChanges: nil, - BackendDefaults: nil, - } -) +var empty = ResourceLifecycleConfig{ + IgnoreRemoteChanges: nil, + IgnoreLocalChanges: nil, + RecreateOnChanges: nil, + UpdateIDOnChanges: nil, + BackendDefaults: nil, +} -// MustLoadConfig loads and parses the embedded resources.yml configuration. -// The config is loaded once and cached for subsequent calls. -// Panics if the embedded YAML is invalid. -func MustLoadConfig() *Config { - configOnce.Do(func() { - globalConfig = &Config{ - Resources: nil, - } - if err := yaml.Unmarshal(resourcesYAML, globalConfig); err != nil { +func mustParseConfig(data []byte) func() *Config { + return sync.OnceValue(func() *Config { + c := &Config{Resources: nil} + if err := yaml.Unmarshal(data, c); err != nil { panic(err) } + return c }) - return globalConfig } -// MustLoadGeneratedConfig loads and parses the embedded resources.generated.yml configuration. -// The config is loaded once and cached for subsequent calls. -// Panics if the embedded YAML is invalid. +var loadConfig = mustParseConfig(resourcesYAML) + +var loadGeneratedConfig = mustParseConfig(resourcesGeneratedYAML) + +// MustLoadConfig returns the parsed resources.yml configuration. +func MustLoadConfig() *Config { + return loadConfig() +} + +// MustLoadGeneratedConfig returns the parsed resources.generated.yml configuration. func MustLoadGeneratedConfig() *Config { - generatedConfigOnce.Do(func() { - generatedConfig = &Config{ - Resources: nil, - } - if err := yaml.Unmarshal(resourcesGeneratedYAML, generatedConfig); err != nil { - panic(err) - } - }) - return generatedConfig + return loadGeneratedConfig() } // GetResourceConfig returns the lifecycle config for a given resource type. diff --git a/internal/build/info.go b/internal/build/info.go index 15967be8ff..54c3f28683 100644 --- a/internal/build/info.go +++ b/internal/build/info.go @@ -42,10 +42,6 @@ func (i Info) GetSanitizedVersion() string { return version } -var info Info - -var once sync.Once - const DefaultSemver = "0.0.0-dev" // getDefaultBuildVersion uses build information stored by Go itself @@ -73,7 +69,7 @@ func getDefaultBuildVersion() string { return out } -func initialize() { +func initialize() Info { // If buildVersion is empty it means the binary was NOT built through goreleaser. // We try to pull version information from debug.BuildInfo(). if buildVersion == "" { @@ -86,7 +82,7 @@ func initialize() { panic(fmt.Sprintf(`version is not a valid semver string: "%s"`, buildVersion)) } - info = Info{ + return Info{ ProjectName: buildProjectName, Version: buildVersion, @@ -106,9 +102,10 @@ func initialize() { } } +var getInfo = sync.OnceValue(initialize) + func GetInfo() Info { - once.Do(initialize) - return info + return getInfo() } func parseInt(s string) int64 { diff --git a/internal/build/variables.go b/internal/build/variables.go index 80c4683aba..f5d8e00b61 100644 --- a/internal/build/variables.go +++ b/internal/build/variables.go @@ -1,5 +1,7 @@ package build +import "sync" + var ( buildProjectName string = "cli" buildVersion string = "" @@ -23,8 +25,8 @@ var ( buildTimestamp string = "0" ) -// This function is used to set the build version for testing purposes. +// SetBuildVersion sets the build version for testing purposes. func SetBuildVersion(version string) { buildVersion = version - info.Version = version + getInfo = sync.OnceValue(initialize) } diff --git a/libs/apps/logstream/streamer.go b/libs/apps/logstream/streamer.go index c916226a71..81624bedb9 100644 --- a/libs/apps/logstream/streamer.go +++ b/libs/apps/logstream/streamer.go @@ -347,15 +347,12 @@ func handleCloseError(err error) (bool, error) { } func watchContext(ctx context.Context, conn *websocket.Conn) func() { - var once sync.Once closeCh := make(chan struct{}) - closeConn := func() { - once.Do(func() { - _ = conn.WriteControl(websocket.CloseMessage, websocket.FormatCloseMessage(websocket.CloseNormalClosure, "context canceled"), time.Now().Add(time.Second)) - _ = conn.Close() - }) - } + closeConn := sync.OnceFunc(func() { + _ = conn.WriteControl(websocket.CloseMessage, websocket.FormatCloseMessage(websocket.CloseNormalClosure, "context canceled"), time.Now().Add(time.Second)) + _ = conn.Close() + }) go func() { select { diff --git a/libs/apps/vite/bridge.go b/libs/apps/vite/bridge.go index 329cb60ef4..ee09d46630 100644 --- a/libs/apps/vite/bridge.go +++ b/libs/apps/vite/bridge.go @@ -83,7 +83,7 @@ type Bridge struct { tunnelID string tunnelWriteChan chan prioritizedMessage stopChan chan struct{} - stopOnce sync.Once + stop func() httpClient *http.Client connectionRequests chan *BridgeMessage port int @@ -101,7 +101,7 @@ func NewBridge(ctx context.Context, w *databricks.WorkspaceClient, appName strin DisableCompression: false, } - return &Bridge{ + b := &Bridge{ ctx: ctx, w: w, appName: appName, @@ -114,6 +114,22 @@ func NewBridge(ctx context.Context, w *databricks.WorkspaceClient, appName strin connectionRequests: make(chan *BridgeMessage, 10), port: port, } + + b.stop = sync.OnceFunc(func() { + close(b.stopChan) + + if b.tunnelConn != nil { + _ = b.tunnelConn.WriteMessage(websocket.CloseMessage, websocket.FormatCloseMessage(websocket.CloseNormalClosure, "")) + b.tunnelConn.Close() + } + + if b.hmrConn != nil { + _ = b.hmrConn.WriteMessage(websocket.CloseMessage, websocket.FormatCloseMessage(websocket.CloseNormalClosure, "")) + b.hmrConn.Close() + } + }) + + return b } func (vb *Bridge) getAuthHeaders(wsURL string) (http.Header, error) { @@ -965,17 +981,5 @@ func (vb *Bridge) Start() error { } func (vb *Bridge) Stop() { - vb.stopOnce.Do(func() { - close(vb.stopChan) - - if vb.tunnelConn != nil { - _ = vb.tunnelConn.WriteMessage(websocket.CloseMessage, websocket.FormatCloseMessage(websocket.CloseNormalClosure, "")) - vb.tunnelConn.Close() - } - - if vb.hmrConn != nil { - _ = vb.hmrConn.WriteMessage(websocket.CloseMessage, websocket.FormatCloseMessage(websocket.CloseNormalClosure, "")) - vb.hmrConn.Close() - } - }) + vb.stop() } diff --git a/libs/cmdio/spinner.go b/libs/cmdio/spinner.go index 503a03ad64..b9bfde46f2 100644 --- a/libs/cmdio/spinner.go +++ b/libs/cmdio/spinner.go @@ -102,11 +102,11 @@ func (m spinnerModel) View() string { // The spinner automatically degrades in non-interactive terminals. // Context cancellation will automatically close the spinner. type spinner struct { - p *tea.Program // nil in non-interactive mode - c *cmdIO - ctx context.Context - once sync.Once - done chan struct{} // Closed when tea.Program finishes + p *tea.Program // nil in non-interactive mode + c *cmdIO + ctx context.Context + sendQuit func() + done chan struct{} // Closed when tea.Program finishes } // Update sends a status message to the spinner. @@ -121,11 +121,7 @@ func (sp *spinner) Update(msg string) { // It waits for the spinner to fully terminate before returning. // It is safe to call Close multiple times and from multiple goroutines. func (sp *spinner) Close() { - sp.once.Do(func() { - if sp.p != nil { - sp.p.Send(quitMsg{}) - } - }) + sp.sendQuit() // Always wait for termination, even if we weren't the first caller if sp.p != nil { <-sp.done @@ -147,7 +143,7 @@ func (sp *spinner) Close() { func (c *cmdIO) NewSpinner(ctx context.Context, opts ...SpinnerOption) *spinner { // Don't show spinner if not interactive if !c.capabilities.SupportsInteractive() { - return &spinner{p: nil, c: c, ctx: ctx} + return &spinner{p: nil, c: c, ctx: ctx, sendQuit: func() {}} } // Create model and program @@ -167,10 +163,11 @@ func (c *cmdIO) NewSpinner(ctx context.Context, opts ...SpinnerOption) *spinner done := make(chan struct{}) sp := &spinner{ - p: p, - c: c, - ctx: ctx, - done: done, + p: p, + c: c, + ctx: ctx, + sendQuit: sync.OnceFunc(func() { p.Send(quitMsg{}) }), + done: done, } // Start program in background From 4194b75fc93e306d3fc9aecd395595ce5f41b3f9 Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Tue, 14 Apr 2026 00:04:50 +0200 Subject: [PATCH 2/2] Update comment on getClient field Co-authored-by: Isaac --- bundle/bundle.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/bundle/bundle.go b/bundle/bundle.go index 93fba5d3d8..f4d33daed1 100644 --- a/bundle/bundle.go +++ b/bundle/bundle.go @@ -120,8 +120,7 @@ type Bundle struct { // in the WSFS location containing the bundle state. Metadata metadata.Metadata - // Store a pointer to the workspace client. - // It can be initialized on demand after loading the configuration. + // Returns the workspace client, initializing it on first call. getClient func() (*databricks.WorkspaceClient, error) // Files that are synced to the workspace.file_path