chore: Simplify Branding (#61905)

We previously required a watcher here that made access to Branding dependent on the watcher init func to be called.
This simplifies that, since the computation of branding is fairly cheap there's no value in memoizing the result.

Test plan:

Sg start looks normal.

cmd/frontend/globals/BUILD.bazel

@@ -7,7 +7,6 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//internal/conf",
-        "//schema",
         "@com_github_inconshreveable_log15//:log15",
     ],
 )

cmd/frontend/globals/globals.go

@@ -10,7 +10,6 @@ import (
 	"github.com/inconshreveable/log15" //nolint:logging // TODO move all logging to sourcegraph/log //nolint:go
 
 	"github.com/sourcegraph/sourcegraph/internal/conf"
-	"github.com/sourcegraph/sourcegraph/schema"
 )
 
 var defaultExternalURL = &url.URL{
@@ -66,61 +65,6 @@ func SetExternalURL(u *url.URL) {
 	externalURL.Store(u)
 }
 
-var defaultBranding = &schema.Branding{
-	BrandName: "Sourcegraph",
-}
-
-// branding mirrors the value of `branding` in the site configuration.
-// This variable is used to monitor configuration change via conf.Watch and must be operated atomically.
-var branding = func() atomic.Value {
-	var v atomic.Value
-	v.Store(defaultBranding)
-	return v
-}()
-
-var brandingWatchers uint32
-
-// WatchBranding watches for changes in the `branding` site configuration
-// so that changes are reflected in what is returned by the Branding function.
-// This should only be called once and will panic otherwise.
-func WatchBranding() {
-	if atomic.AddUint32(&brandingWatchers, 1) != 1 {
-		panic("WatchBranding called more than once")
-	}
-
-	conf.Watch(func() {
-		after := conf.Get().Branding
-		if after == nil {
-			after = defaultBranding
-		} else if after.BrandName == "" {
-			bcopy := *after
-			bcopy.BrandName = defaultBranding.BrandName
-			after = &bcopy
-		}
-
-		if before := Branding(); !reflect.DeepEqual(before, after) {
-			SetBranding(after)
-			log15.Debug(
-				"globals.Branding",
-				"updated", true,
-				"before", before,
-				"after", after,
-			)
-		}
-	})
-}
-
-// Branding returns the last valid value of branding in the site configuration.
-// Callers must not mutate the returned pointer.
-func Branding() *schema.Branding {
-	return branding.Load().(*schema.Branding)
-}
-
-// SetBranding sets a valid value for the branding.
-func SetBranding(u *schema.Branding) {
-	branding.Store(u)
-}
-
 // ConfigurationServerFrontendOnly provides the contents of the site configuration
 // to other services and manages modifications to it.
 //

cmd/frontend/internal/app/jscontext/jscontext.go

@@ -399,7 +399,7 @@ func NewJSContextFromRequest(req *http.Request, db database.DB) JSContext {
 
 		AuthAccessRequest: conf.Get().AuthAccessRequest,
 
-		Branding: globals.Branding(),
+		Branding: conf.Branding(),
 
 		BatchChangesEnabled:                enterprise.BatchChangesEnabledForUser(ctx, db) == nil,
 		BatchChangesDisableWebhooksWarning: conf.Get().BatchChangesDisableWebhooksWarning,

cmd/frontend/internal/app/misc_handlers.go

@@ -10,8 +10,8 @@ import (
 
 	"github.com/gorilla/mux"
 
-	"github.com/sourcegraph/sourcegraph/cmd/frontend/globals"
 	"github.com/sourcegraph/sourcegraph/cmd/frontend/internal/app/assetsutil"
+	"github.com/sourcegraph/sourcegraph/internal/conf"
 	"github.com/sourcegraph/sourcegraph/internal/conf/deploy"
 	"github.com/sourcegraph/sourcegraph/internal/dotcom"
 	"github.com/sourcegraph/sourcegraph/internal/env"
@@ -59,7 +59,7 @@ func favicon(w http.ResponseWriter, r *http.Request) {
 	url.RawQuery = query.Encode()
 	path := strings.Replace(url.String(), "v2=", "v2", 1)
 
-	if branding := globals.Branding(); branding.Favicon != "" {
+	if branding := conf.Branding(); branding.Favicon != "" {
 		path = branding.Favicon
 	}
 	http.Redirect(w, r, path, http.StatusMovedPermanently)

cmd/frontend/internal/app/ui/BUILD.bazel

@@ -26,7 +26,6 @@ go_library(
         "//cmd/frontend/auth",
         "//cmd/frontend/backend",
         "//cmd/frontend/envvar",
-        "//cmd/frontend/globals",
         "//cmd/frontend/hubspot",
         "//cmd/frontend/hubspot/hubspotutil",
         "//cmd/frontend/internal/app/assetsutil",

cmd/frontend/internal/app/ui/handlers.go

@@ -19,7 +19,6 @@ import (
 	"github.com/sourcegraph/sourcegraph/cmd/frontend/auth"
 	"github.com/sourcegraph/sourcegraph/cmd/frontend/backend"
 	"github.com/sourcegraph/sourcegraph/cmd/frontend/envvar"
-	"github.com/sourcegraph/sourcegraph/cmd/frontend/globals"
 	"github.com/sourcegraph/sourcegraph/cmd/frontend/hubspot"
 	"github.com/sourcegraph/sourcegraph/cmd/frontend/hubspot/hubspotutil"
 	"github.com/sourcegraph/sourcegraph/cmd/frontend/internal/app/assetsutil"
@@ -153,7 +152,8 @@ func newCommon(w http.ResponseWriter, r *http.Request, db database.DB, title str
 
 	var preloadedAssets *[]PreloadedAsset
 	preloadedAssets = nil
-	if globals.Branding() == nil || (globals.Branding().Dark == nil && globals.Branding().Light == nil) {
+	br := conf.Branding()
+	if br == nil || (br.Dark == nil && br.Light == nil) {
 		preloadedAssets = &[]PreloadedAsset{
 			// sourcegraph-mark.svg is always loaded as part of the layout component unless a custom
 			// branding is defined
@@ -173,7 +173,7 @@ func newCommon(w http.ResponseWriter, r *http.Request, db database.DB, title str
 		Manifest:        manifest,
 		PreloadedAssets: preloadedAssets,
 		Metadata: &Metadata{
-			Title:       globals.Branding().BrandName,
+			Title:       br.BrandName,
 			Description: "Sourcegraph is a web-based code search and navigation tool for dev teams. Search, navigate, and review code. Find answers.",
 			ShowPreview: r.URL.Path == "/sign-in" && r.URL.RawQuery == "returnTo=%2F",
 		},
@@ -336,7 +336,7 @@ func serveBasicPage(db database.DB, title func(c *Common, r *http.Request) strin
 
 func serveHome(db database.DB) handlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) error {
-		common, err := newCommon(w, r, db, globals.Branding().BrandName, index, serveError)
+		common, err := newCommon(w, r, db, conf.Branding().BrandName, index, serveError)
 		if err != nil {
 			return err
 		}

cmd/frontend/internal/app/ui/raw.go

@@ -20,7 +20,7 @@ import (
 
 	"github.com/sourcegraph/log"
 
-	"github.com/sourcegraph/sourcegraph/cmd/frontend/globals"
+	"github.com/sourcegraph/sourcegraph/internal/conf"
 	"github.com/sourcegraph/sourcegraph/internal/database"
 	"github.com/sourcegraph/sourcegraph/internal/errcode"
 	"github.com/sourcegraph/sourcegraph/internal/gitserver"
@@ -75,7 +75,7 @@ func serveRaw(logger log.Logger, db database.DB, gitserverClient gitserver.Clien
 		// - Gitserver content updating
 		// - Consistent error handling (permissions, revision not found, repo not found, etc).
 		//
-		common, err := newCommon(w, r, db, globals.Branding().BrandName, noIndex, serveError)
+		common, err := newCommon(w, r, db, conf.Branding().BrandName, noIndex, serveError)
 		if err != nil {
 			return err
 		}

cmd/frontend/internal/app/ui/router.go

@@ -15,7 +15,6 @@ import (
 	"github.com/sourcegraph/log"
 	"go.opentelemetry.io/otel/attribute"
 
-	"github.com/sourcegraph/sourcegraph/cmd/frontend/globals"
 	uirouter "github.com/sourcegraph/sourcegraph/cmd/frontend/internal/app/ui/router"
 	"github.com/sourcegraph/sourcegraph/cmd/frontend/internal/githubapp"
 	"github.com/sourcegraph/sourcegraph/cmd/frontend/internal/routevar"
@@ -194,7 +193,7 @@ func InitRouter(db database.DB) {
 		Handler(handler(db, serveBasicPage(db, func(_ *Common, r *http.Request) string {
 			shortQuery := limitString(r.URL.Query().Get("q"), 25, true)
 			if shortQuery == "" {
-				return globals.Branding().BrandName
+				return conf.Branding().BrandName
 			}
 			// e.g. "myquery - Sourcegraph"
 			return brandNameSubtitle(shortQuery)
@@ -322,7 +321,7 @@ var mockServeRepo func(w http.ResponseWriter, r *http.Request)
 // last title component. This function indirectly calls conf.Get(), so should not be invoked from
 // any function that is invoked by an init function.
 func brandNameSubtitle(titles ...string) string {
-	return strings.Join(append(titles, globals.Branding().BrandName), " - ")
+	return strings.Join(append(titles, conf.Branding().BrandName), " - ")
 }
 
 // staticRedirectHandler returns an HTTP handler that redirects all requests to

cmd/frontend/internal/cli/serve_cmd.go

@@ -206,7 +206,6 @@ func Main(ctx context.Context, observationCtx *observation.Context, ready servic
 		return err
 	}
 
-	globals.WatchBranding()
 	globals.WatchExternalURL()
 
 	goroutine.Go(func() { bg.CheckRedisCacheEvictionPolicy() })

internal/conf/computed.go

@@ -1293,3 +1293,17 @@ func PermissionsUserMapping() *schema.PermissionsUserMapping {
 	}
 	return c
 }
+
+func Branding() *schema.Branding {
+	br := Get().Branding
+	if br == nil {
+		br = &schema.Branding{
+			BrandName: "Sourcegraph",
+		}
+	} else if br.BrandName == "" {
+		bcopy := *br
+		bcopy.BrandName = "Sourcegraph"
+		br = &bcopy
+	}
+	return br
+}