artin/sourcegraph
1
0
Fork 0
mirror of https://github.com/sourcegraph/sourcegraph.git synced 2026-02-06 12:51:55 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat(frontend): do not embed frontend assets anymore (#63946)

Browse Source 
* Frontend no longer embeds the assets intead it reads from the local
filesystem assets.
* Generally the frontend and server cmd targets will use the
`//client/web/dist:copy_bundle` target to create a tarball for the
oci_image. `copy_bundle` puts all the assets at `assets-dist`
* For integration tests, frontend and server have the `no_client_bundle`
target variants. For these oci_images, instead of the `tar_bundle` which
is just a tar'd `copy_bundle` we use the `tar_dummy_manifest` which is
just a tar that contains a dummy manifest.
* By default we expect assets to be at `/assets-dist`
* Renamed DevProvider to DirProvider

## Why
By 'breaking' the dependency of frontend requiring assets to be built we
essentially stop a common cache invalidation scenario that happens:
- someone makes a frontend change = assets need to be rebuilt

By decoupling assets from the frontend binary and moving the packing of
assets to the building of the frontend and server images we will have a
better cache hit rate (theoretically).

Thus with this change, when:
* client/web is change and nothing else ... only assets will have to
rebuilt and cached versions of the backend will be used
* if only backend code has changed ... cached assets will be used

Closes DINF-115

## Test plan
✅  sg start - web app opens and can search. Local dev assets get loaded
✅ sg test bazel-integration-test - server image gets built with **only**
dummy web manifest. Also verified by running `sg bazel run
//cmd/server:no_client_bundle.image` and then inspect container
✅ sg test bazel-e2e - server image gets built with bundle and all tests
pass
✅ [main dry
run](https://buildkite.com/sourcegraph/sourcegraph/builds/284042#0190e54c-14d9-419e-95ca-1198dc682048)

## Changelog
- frontend: assets are no longer bundled with binary through `go:embed`.
Instead assets are now added to the frontend container at `assets-dist`.
This commit is contained in:
William Bezuidenhout 2024-07-31 15:17:52 +02:00 committed by GitHub
parent 1538e42180
commit 972ee32351
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
15 changed files with 153 additions and 285 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
BUILD.bazel
View File

@ -1,4 +1,4 @@
load("@bazel_skylib//rules:common_settings.bzl", "bool_flag", "bool_setting")
load("@bazel_skylib//rules:common_settings.bzl", "bool_flag")
load("@io_bazel_rules_go//go:def.bzl", "go_library", "nogo")
load("@aspect_bazel_lib//lib:copy_to_bin.bzl", "copy_to_bin")
load("@aspect_rules_ts//ts:defs.bzl", "ts_config")
@ -271,17 +271,6 @@ go_proto_compiler(
],
)
# Settings for automatic building of frontend/single-server without client bundle included
bool_setting(
name = "integration_testing",
build_setting_default = False,
)
config_setting(
name = "integration_testing_enabled",
flag_values = {":integration_testing": "true"},
)
# nogo config
#
# For nogo to be able to run a linter, it needs to have `var Analyzer analysis.Analyzer` defined in the main package.

46
client/web/dist/BUILD.bazel vendored
View File

@ -1,24 +1,7 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library")
load("@aspect_bazel_lib//lib:copy_to_directory.bzl", "copy_to_directory")
load("@rules_pkg//:pkg.bzl", "pkg_tar")
go_library(
name = "dist",
srcs = ["assets.go"],
embedsrcs = select({
"//conditions:default": [":copy_bundle"],
"//:integration_testing_enabled": [":dummy_web_manifest"],
}), # keep
importpath = "github.com/sourcegraph/sourcegraph/client/web/dist",
visibility = ["//visibility:public"],
x_defs = select({
"//conditions:default": {},
"//:integration_testing_enabled": {"github.com/sourcegraph/sourcegraph/client/web/dist.assetsSubdir": "integration"},
}),
deps = [
"//lib/errors",
"//ui/assets",
],
)
ASSETS_DIR = "assets-dist"
copy_to_directory(
name = "copy_bundle",
@ -29,7 +12,7 @@ copy_to_directory(
"//client/web-sveltekit",
"//client/web/dist/img",
],
out = "dist",
out = ASSETS_DIR,
replace_prefixes = {
"client/web/bundle": "",
"client/web-sveltekit/build": "",
@ -40,10 +23,31 @@ copy_to_directory(
"client/browser/integration-assets/extension/js/extensionHostWorker.bundle.js": "extension/scripts/extensionHostWorker.bundle.js",
"client/browser/integration-assets": "",
},
visibility = ["//visibility:public"],
)
genrule(
name = "dummy_web_manifest",
outs = ["integration/web.manifest.json"],
outs = ["web.manifest.json"],
cmd = "echo '{}' > $@",
visibility = ["//visibility:public"],
)
pkg_tar(
name = "tar_dummy_manifest",
srcs = [
":dummy_web_manifest",
],
mode = "0655",
package_dir = ASSETS_DIR,
visibility = ["//visibility:public"],
)
pkg_tar(
name = "tar_bundle",
srcs = [
":copy_bundle",
],
mode = "0655",
visibility = ["//visibility:public"],
)

81
client/web/dist/assets.go vendored
View File

@ -1,81 +0,0 @@
package dist
import (
"embed"
"encoding/json"
"io"
"io/fs"
"net/http"
"sync"
"github.com/sourcegraph/sourcegraph/lib/errors"
"github.com/sourcegraph/sourcegraph/ui/assets"
)
// By default, `go:embed *` will ignore files that start with `.` or `_`. `all:*` on the other
// hand will truly include all files.
//
//go:embed all:*
var assetsFS embed.FS
var afs fs.FS = assetsFS
var Assets http.FileSystem
var assetsSubdir = "dist"
var (
webBuildManifestOnce sync.Once
assetsOnce sync.Once
webBuildManifest *assets.WebBuildManifest
webBuildManifestErr error
)
func init() {
// Sets the global assets provider.
assets.Provider = Provider{}
}
type Provider struct{}
func (p Provider) LoadWebBuildManifest() (*assets.WebBuildManifest, error) {
webBuildManifestOnce.Do(func() {
f, err := afs.Open("web.manifest.json")
if err != nil {
webBuildManifestErr = errors.Wrap(err, "read manifest file")
return
}
defer f.Close()
manifestContent, err := io.ReadAll(f)
if err != nil {
webBuildManifestErr = errors.Wrap(err, "read manifest file")
return
}
if err := json.Unmarshal(manifestContent, &webBuildManifest); err != nil {
webBuildManifestErr = errors.Wrap(err, "unmarshal manifest json")
return
}
})
return webBuildManifest, webBuildManifestErr
}
func (p Provider) Assets() http.FileSystem {
assetsOnce.Do(func() {
// When we're building this package with Bazel, we cannot directly output the files in this current folder, because
// it's already containing other files known to Bazel. So instead we put those into the dist folder.
// If we do detect a dist folder when running this code, we immediately substitute the root to that dist folder.
//
// Therefore, this code works with both the traditionnal build approach and when built with Bazel.
if _, err := assetsFS.ReadDir(assetsSubdir); err == nil {
var err error
afs, err = fs.Sub(assetsFS, assetsSubdir)
if err != nil {
panic("incorrect embed")
}
}
Assets = http.FS(afs)
})
return Assets
}

30
cmd/frontend/BUILD.bazel
View File

@ -2,7 +2,6 @@ load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library")
load("//dev:oci_defs.bzl", "image_repository", "oci_image", "oci_push", "oci_tarball")
load("@rules_pkg//:pkg.bzl", "pkg_tar")
load("@container_structure_test//:defs.bzl", "container_structure_test")
load("//cmd/server:macro.bzl", "go_binary_nobundle")
go_library(
name = "frontend_lib",
@ -10,9 +9,9 @@ go_library(
importpath = "github.com/sourcegraph/sourcegraph/cmd/frontend",
visibility = ["//visibility:private"],
deps = [
"//client/web/dist",
"//cmd/frontend/shared",
"//internal/sanitycheck",
"//ui/assets",
],
)
@ -24,7 +23,10 @@ go_binary(
pkg_tar(
name = "tar_frontend",
srcs = [":frontend"],
srcs = [
":frontend",
],
visibility = ["//visibility:public"],
)
oci_image(
@ -54,6 +56,7 @@ oci_image(
# Needed so the "Upgrade readiness" page in the site-admin
# can find locally the current schema.
"//cmd/migrator:tar_current_schema_descriptions",
"//client/web/dist:tar_bundle",
],
user = "sourcegraph",
)
@ -85,24 +88,6 @@ oci_push(
# No client bundle targets
go_binary_nobundle(
name = "frontend_nobundle",
# We can't set `out` here to `frontend` because it would conflict with the
# :frontend target's outputs, hence we remap the path in the `pkg_tar` as part of
# creating the image.
# file 'cmd/frontend/frontend' is generated by these conflicting actions:
# Label: //cmd/frontend:frontend_nobundle_underlying, //cmd/frontend:frontend_nobundle
embed = [":frontend_lib"],
visibility = ["//visibility:public"],
)
pkg_tar(
name = "no_client_bundle.tar",
srcs = [":frontend_nobundle"],
remap_paths = {"/frontend_nobundle": "/frontend"},
visibility = ["//visibility:public"],
)
oci_image(
name = "no_client_bundle.image",
base = "//wolfi-images/sourcegraph-base:base_image",
@ -126,10 +111,11 @@ oci_image(
"CODEINTEL_PGUSER": "sg",
},
tars = [
":no_client_bundle.tar",
":tar_frontend",
# Needed so the "Upgrade readiness" page in the site-admin
# can find locally the current schema.
"//cmd/migrator:tar_current_schema_descriptions",
"//client/web/dist:tar_dummy_manifest",
],
user = "sourcegraph",
)

3
cmd/frontend/image_test.yaml
View File

@ -30,6 +30,9 @@ fileExistenceTests:
shouldExist: true
uid: 0
gid: 0
- name: '/assets-dist/web.manifest.json'
path: '/assets-dist/web.manifest.json'
shouldExist: true
metadataTest:
envVars:

4
cmd/frontend/main.go
View File

@ -4,11 +4,11 @@ package main
import (
"github.com/sourcegraph/sourcegraph/cmd/frontend/shared"
"github.com/sourcegraph/sourcegraph/internal/sanitycheck"
_ "github.com/sourcegraph/sourcegraph/client/web/dist" // use assets
"github.com/sourcegraph/sourcegraph/ui/assets"
)
func main() {
assets.Init()
sanitycheck.Pass()
shared.FrontendMain(nil)
}

51
cmd/server/BUILD.bazel
View File

@ -4,7 +4,6 @@ load("@container_structure_test//:defs.bzl", "container_structure_test")
load("@rules_pkg//:pkg.bzl", "pkg_tar")
load("macro.bzl", "container_dependencies", "dependencies_tars")
load("//wolfi-images:defs.bzl", "wolfi_base")
load("//cmd/server:macro.bzl", "go_binary_nobundle")
go_library(
name = "server_lib",
@ -12,9 +11,9 @@ go_library(
importpath = "github.com/sourcegraph/sourcegraph/cmd/server",
visibility = ["//visibility:private"],
deps = [
"//client/web/dist",
"//cmd/server/shared",
"//internal/sanitycheck",
"//ui/assets",
],
)
@ -106,9 +105,9 @@ ZOEKT_DEPS = [
]
# Declares rules for pkg_tar for each dep in DEPS
container_dependencies(DEPS)
container_dependencies(targets = DEPS)
container_dependencies(ZOEKT_DEPS)
container_dependencies(targets = ZOEKT_DEPS)
pkg_tar(
name = "tar_syntax-highlighter",
@ -178,8 +177,9 @@ oci_image(
":tar_postgres_optimize",
":tar_postgres_reindex",
":tar_prom-wrapper",
":tar_image_test_scripts",
"//client/web/dist:tar_bundle",
"//monitoring:generate_grafana_config_tar",
"tar_image_test_scripts",
] + dependencies_tars(DEPS) + dependencies_tars(ZOEKT_DEPS),
workdir = "/",
)
@ -213,30 +213,6 @@ wolfi_base()
# No client bundle targets
go_binary_nobundle(
name = "server_nobundle",
embed = [":server_lib"],
gotags = [
"netgo",
"dist",
],
pure = "on",
visibility = ["//visibility:public"],
)
pkg_tar(
name = "no_client_bundle.tar",
srcs = [":server_nobundle"],
remap_paths = {"/server_nobundle": "/server"},
)
#
pkg_tar(
name = "no_client_bundle_frontend.tar",
srcs = ["//cmd/frontend:frontend_nobundle"],
remap_paths = {"/frontend_nobundle": "/usr/local/bin/frontend"},
)
oci_image(
name = "no_client_bundle.image",
base = ":base_image",
@ -252,7 +228,7 @@ oci_image(
# "PGHOST": "/var/run/postgresql",
},
tars = [
":no_client_bundle.tar",
":tar_server",
":static_config_tar",
":tar_postgres_exporter_config",
":tar_monitoring_config",
@ -261,19 +237,10 @@ oci_image(
":tar_postgres_optimize",
":tar_postgres_reindex",
":tar_prom-wrapper",
"//monitoring:generate_grafana_config_tar",
"tar_image_test_scripts",
"no_client_bundle_frontend.tar",
] + dependencies_tars([
"//cmd/precise-code-intel-worker",
"//cmd/searcher",
"//cmd/embeddings",
"//cmd/gitserver",
"//cmd/migrator",
"//cmd/repo-updater",
"//cmd/symbols",
"//cmd/worker",
]) + dependencies_tars(ZOEKT_DEPS),
"//monitoring:generate_grafana_config_tar",
"//client/web/dist:tar_dummy_manifest",
] + dependencies_tars(DEPS) + dependencies_tars(ZOEKT_DEPS),
workdir = "/",
)

3
cmd/server/image_test.yaml
View File

@ -112,6 +112,9 @@ fileExistenceTests:
shouldExist: true
uid: 0
gid: 0
- name: '/assets-dist/web.manifest.json'
path: '/assets-dist/web.manifest.json'
shouldExist: true
metadataTest:
envVars:

72
cmd/server/macro.bzl
View File

@ -1,7 +1,18 @@
"""
Various helpers to help with server image building
"""
load("@rules_pkg//:pkg.bzl", "pkg_tar")
load("@io_bazel_rules_go//go:def.bzl", "GoArchive", "go_binary")
def get_last_segment(path):
"""
returns part of a bazel path - it will return binary in //something:binary
Args:
path: the path from which the last segment should be extract from
Returns:
Returns the last part found after `:`. If no `:` is found then the last part after the last '/' is returned
"""
segments = path.split("/")
last_segment = segments[-1]
@ -12,6 +23,15 @@ def get_last_segment(path):
return s[-1]
def container_dependencies(targets):
"""
creates pkg_tar rules for all given targets
for all the given targets this will create a pkg_tar rule named 'tar_<name>` where
the target is added as well as the path of the target output is remapped to be at /usr/local/bin
Args:
targets: list of targets for which pkg_tar rules should be generated for
"""
for target in targets:
name = get_last_segment(target)
@ -22,51 +42,17 @@ def container_dependencies(targets):
)
def dependencies_tars(targets):
"""
for all the given targets it returns a list of the corresponding `:tar_<name>` targets
Args:
targets: list of targets
Returns:
list of ':tar_<name>' targets
"""
tars = []
for target in targets:
name = get_last_segment(target)
tars.append(":tar_{}".format(name))
return tars
def go_binary_nobundle(name, **kwargs):
go_binary(
name = name + "_underlying",
out = kwargs.pop("out", name + "_underlying"),
**kwargs
)
go_binary_nobundle_rule(
name = name,
go_binary = ":" + name + "_underlying",
out = native.package_name() + "/" + name,
visibility = kwargs.pop("visibility", ["//visibility:public"]),
)
def _go_binary_nobundle_rule(ctx):
# so that we can `bazel run` nobundle targets, we need to set `executable` in DefaultInfo.
# But this can't be the output of a _different_ rule, it has to be the output of this rule.
executable = ctx.actions.declare_file(ctx.attr.out)
ctx.actions.symlink(output = executable, target_file = ctx.executable.go_binary)
return [DefaultInfo(executable = executable, files = depset(direct = [executable]))]
go_binary_nobundle_rule = rule(
implementation = _go_binary_nobundle_rule,
executable = True,
attrs = {
"go_binary": attr.label(
providers = [GoArchive],
executable = True,
mandatory = True,
cfg = transition(
implementation = lambda settings, attr: [{"//:integration_testing": True}],
inputs = [],
outputs = ["//:integration_testing"],
),
),
"out": attr.string(mandatory = True),
"_allowlist_function_transition": attr.label(
default = "@bazel_tools//tools/allowlists/function_transition_allowlist",
),
},
)

3
cmd/server/main.go
View File

@ -7,10 +7,11 @@ import (
"github.com/sourcegraph/sourcegraph/cmd/server/shared"
"github.com/sourcegraph/sourcegraph/internal/sanitycheck"
_ "github.com/sourcegraph/sourcegraph/client/web/dist" // use assets
"github.com/sourcegraph/sourcegraph/ui/assets"
)
func main() {
assets.Init()
sanitycheck.Pass()
enableEmbeddings, _ := strconv.ParseBool(os.Getenv("SRC_ENABLE_EMBEDDINGS"))

1
ui/assets/.gitignore vendored
View File

@ -11,5 +11,6 @@
stats.json
*.hot-update.json
_sk/
!dist/
web.manifest.json
webpack.manifest.json # legacy, can be removed after 2023-12-01

5
ui/assets/BUILD.bazel
View File

@ -4,15 +4,10 @@ go_library(
name = "assets",
srcs = [
"assets.go",
"dev.go",
"doc.go",
"manifest.go",
],
importpath = "github.com/sourcegraph/sourcegraph/ui/assets",
visibility = ["//visibility:public"],
x_defs = select({
"//conditions:default": {},
"//:integration_testing_enabled": {"github.com/sourcegraph/sourcegraph/client/web/dist.assetsSubdir": "integration"},
}),
deps = ["//lib/errors"],
)

61
ui/assets/assets.go
View File

@ -1,11 +1,25 @@
package assets
import (
"encoding/json"
"net/http"
"os"
"path/filepath"
"github.com/sourcegraph/sourcegraph/lib/errors"
)
// DefaultAssetPath is the default path where assets should be loaded from
// See //client/web/dist:copy_bundle where assets are copied to this directory
const DefaultAssetPath = "/assets-dist"
// DevAssetsPath is the path to the assets directory when we're in development mode
const DevAssetsPath = "client/web/dist"
func Init() {
UseAssetsProviderForPath(DefaultAssetPath)
}
// AssetsProvider abstracts accessing assets and the web build manifest.
// One implementation must be explicitly set in the main.go using
// this code. See ui/assets/doc.go
@ -33,3 +47,50 @@ func (p FailingAssetsProvider) LoadWebBuildManifest() (*WebBuildManifest, error)
func (p FailingAssetsProvider) Assets() http.FileSystem {
panic("assets are not configured for this binary, please see ui/assets/doc.go")
}
// UseDevAssetsProvider installs the development variant of the DirProvider
// which expects assets to be generated on the fly by an external web builder process.
func UseDevAssetsProvider() {
// When we're using the dev asset provider we expect to be in the monorepo, therefore we use a relative path
UseAssetsProviderForPath(DevAssetsPath)
}
// UseAssetsProviderForPath sets the global Provider to a DirProvider using the given path
func UseAssetsProviderForPath(path string) {
Provider = DirProvider{dir: path, assets: http.Dir(path)}
}
// DirProvider implements the AssetsProvider interface and loads assets from a directory
type DirProvider struct {
dir string
assets http.FileSystem
}
func (p DirProvider) LoadWebBuildManifest() (*WebBuildManifest, error) {
return loadWebBuildManifest(p.dir)
}
func (p DirProvider) Assets() http.FileSystem {
return p.assets
}
var MockLoadWebBuildManifest func() (*WebBuildManifest, error)
// loadWebBuildManifest uses a web builder manifest to extract hashed bundle names to
// serve to the client. In dev mode, we load this file from disk on demand, so it doesn't
// have to exist at compile time, to avoid a build dependency between frontend
// and client.
func loadWebBuildManifest(rootDir string) (m *WebBuildManifest, err error) {
if MockLoadWebBuildManifest != nil {
return MockLoadWebBuildManifest()
}
manifestContent, err := os.ReadFile(filepath.Join(rootDir, "web.manifest.json"))
if err != nil {
return nil, errors.Wrapf(err, "failed loading web build manifest file from disk at %q", rootDir)
}
if err := json.Unmarshal(manifestContent, &m); err != nil {
return nil, errors.Wrap(err, "parsing manifest json")
}
return m, nil
}

53
ui/assets/dev.go
View File

@ -1,53 +0,0 @@
package assets
import (
"encoding/json"
"net/http"
"os"
"path/filepath"
"github.com/sourcegraph/sourcegraph/lib/errors"
)
const assetsDir = "client/web/dist"
// UseDevAssetsProvider installs the development variant of the UseDevAssetsProvider
// which expects assets to be generated on the fly by an external web builder process.
func UseDevAssetsProvider() {
Provider = DevProvider{assets: http.Dir(assetsDir)}
}
// DevProvider is the development variant of the UseDevAssetsProvider
// which expects assets to be generated on the fly by an external web builder process.
type DevProvider struct {
assets http.FileSystem
}
func (p DevProvider) LoadWebBuildManifest() (*WebBuildManifest, error) {
return loadWebBuildManifest()
}
func (p DevProvider) Assets() http.FileSystem {
return p.assets
}
var MockLoadWebBuildManifest func() (*WebBuildManifest, error)
// loadWebBuildManifest uses a web builder manifest to extract hashed bundle names to
// serve to the client. In dev mode, we load this file from disk on demand, so it doesn't
// have to exist at compile time, to avoid a build dependency between frontend
// and client.
func loadWebBuildManifest() (m *WebBuildManifest, err error) {
if MockLoadWebBuildManifest != nil {
return MockLoadWebBuildManifest()
}
manifestContent, err := os.ReadFile(filepath.Join(assetsDir, "web.manifest.json"))
if err != nil {
return nil, errors.Wrap(err, "loading web build manifest file from disk")
}
if err := json.Unmarshal(manifestContent, &m); err != nil {
return nil, errors.Wrap(err, "parsing manifest json")
}
return m, nil
}

12
ui/assets/doc.go
View File

@ -4,20 +4,26 @@
// seeking to provide access to assets, regardless of their type (dev, oss
// or enterprise).
// You must also import the embedded assets:
// Before using assets you have to call the package Init function, which sets up the global Provider
// with the path to the assets, which by default is /assets-dist
//
// import _ "github.com/sourcegraph/sourcegraph/client/web/dist"
// import "github.com/sourcegraph/sourcegraph/ui/assets"
//
// func main() {
// assets.Init()
// }
//
// And to support working with dev assets, with the web builder process handling them for you, you can use:
//
// func main() {
// assets.Init()
// if os.Getenv("WEB_BUILDER_DEV_SERVER") == "1" {
// assets.UseDevAssetsProvider()
// }
// // ...
// }
//
// If this step isn't done, the default assets provider implementation, FailingAssetsProvider will ensure
// If `assets.Init()` isn't called, the default assets provider implementation, FailingAssetsProvider will ensure
// the binary panics when launched and will explicitly tell you about the problem.
//
// This enables to express which bundle type is needed at compile time, expressed through package dependency,