diff --git a/cmd/symbols/flutter_sources.go b/cmd/symbols/flutter_sources.go new file mode 100644 index 00000000..919cfaea --- /dev/null +++ b/cmd/symbols/flutter_sources.go @@ -0,0 +1,306 @@ +package symbols + +import ( + "bytes" + "fmt" + "io/fs" + "os" + "path" + "path/filepath" + "sort" + "strings" + + "github.com/launchdarkly/ldcli/internal/symbols/flutter" + "github.com/launchdarkly/ldcli/internal/symbols/srcbundle" +) + +// Flutter source bundling. +// +// Dart names a compilation unit by its script URI, so what a .dartmap stores as a +// frame's file — and therefore what the bundle has to be keyed by — is rarely a +// path. It is "package:my_app/main.dart" for the app's own code, "dart:async" or +// "org-dartlang-sdk:///..." for the SDK, "package:/..." for a dependency, +// and only sometimes a plain or file:// path. +// +// Keys are always the URI exactly as the map spells it, because that is what the +// backend looks a frame up by. Which file on this machine backs a key is a +// separate question, answered here: a package URI resolves through the app's +// pubspec name to lib/, and anything belonging to the SDK or to a dependency is +// left out rather than guessed at. + +// flutterSourceBundleName is the object name of the source bundle uploaded beside a +// build's .dartmap, so a map and the sources behind it share one key prefix: +// _sym/flutter/id//app.dartmap and .../sources.srcbundle. +const flutterSourceBundleName = "sources.srcbundle" + +// flutterSourceExtensions are the file types the UI can render as Dart source context. +var flutterSourceExtensions = map[string]bool{ + ".dart": true, +} + +// flutterVendorSchemes are the URI schemes naming code that ships with Dart or +// Flutter rather than being written by the developer. +var flutterVendorSchemes = map[string]bool{ + "dart": true, + "org-dartlang-sdk": true, + "org-dartlang-app": true, + "org-dartlang-untranslatable-uri": true, +} + +// flutterVendorPathMarkers appear mid-path in Flutter/Dart SDK and pub-cache +// trees, for the builds whose DWARF records real paths instead of package URIs. +var flutterVendorPathMarkers = []string{ + "/.pub-cache/", + "/pub-cache/", + "/flutter/packages/flutter/", + "/flutter/packages/flutter_test/", + "/flutter/packages/flutter_driver/", + "/flutter/packages/flutter_localizations/", + "/flutter/packages/flutter_web_plugins/", + "/flutter/bin/cache/", + "/third_party/dart/", + "/hosted/pub.dev/", + "/hosted/pub.dartlang.org/", +} + +// isFlutterVendorSource reports whether a filesystem path belongs to the +// Flutter/Dart SDK or the pub cache rather than to the project being uploaded. +func isFlutterVendorSource(p string) bool { + lower := strings.ToLower(filepath.ToSlash(p)) + for _, marker := range flutterVendorPathMarkers { + if strings.Contains(lower, marker) { + return true + } + } + return false +} + +// buildFlutterSourceBundle packs the project's .dart sources referenced by the +// images' DWARF into a .srcbundle, keyed by the URI each one is recorded under so +// a resolved frame's FileName is the lookup key. +// +// Only the app's own code is packed. The SDK and every dependency are excluded, +// which matters for more than upload size: a bundle that answered +// "package:flutter/src/material/ink_well.dart" with whatever local file happened +// to share its name would put the wrong code behind a real frame. +// +// Returns nil when nothing was found, so the caller can skip the upload. +func buildFlutterSourceBundle(images []flutter.Image, sourceRoot string) ([]byte, int, error) { + // Merge the arches: one release's Dart sources are identical across them, and + // a crash can arrive from any, so every lane gets the same bundle. + merged := make(map[string]string) + for _, img := range images { + for key, resolved := range img.Sources { + if _, ok := merged[key]; !ok { + merged[key] = resolved + } + } + } + + appPkg := flutterAppPackage(sourceRoot) + byBase := indexDartFilesByBase(sourceRoot) + + // Sorted so the same build always produces the same bundle, including which + // files are dropped if the size budget runs out. + keys := make([]string, 0, len(merged)) + for key := range merged { + keys = append(keys, key) + } + sort.Strings(keys) + + b := &srcbundle.Builder{} + total := 0 + for _, key := range keys { + if !flutterSourceExtensions[strings.ToLower(path.Ext(flutterURIPath(key)))] { + continue + } + local, ok := flutterSourceFile(key, merged[key], appPkg, sourceRoot, byBase) + if !ok { + continue + } + data, err := os.ReadFile(local) + if err != nil { + continue // built elsewhere: the backend renders the frame without source + } + if len(data) > maxSourceFileBytes || total+len(data) > maxSourceBundleBytes { + continue + } + total += len(data) + b.Add(key, data) + } + if b.Len() == 0 { + return nil, 0, nil + } + + var buf bytes.Buffer + if err := b.Encode(&buf); err != nil { + return nil, 0, fmt.Errorf("failed to encode Flutter source bundle: %w", err) + } + return buf.Bytes(), b.Len(), nil +} + +// flutterSourceFile returns the file on this machine to pack under key, or +// ok=false when key names code that is not the project's or cannot be found. +// +// resolved is what the DWARF reader made of the name: a path for the plain and +// file:// forms, the URI itself otherwise. +func flutterSourceFile(key, resolved, appPkg, sourceRoot string, byBase map[string][]string) (string, bool) { + scheme, rest, isURI := strings.Cut(key, ":") + if isURI && flutterURIScheme(key) != "" { + if flutterVendorSchemes[strings.ToLower(scheme)] { + return "", false + } + if strings.EqualFold(scheme, "package") { + // package:/ is 's lib/. Only the app's own + // package can be resolved from the project root; a dependency's + // lives in the pub cache and is not ours to upload. + pkg, within, found := strings.Cut(strings.TrimPrefix(rest, "//"), "/") + if !found || appPkg == "" || !strings.EqualFold(pkg, appPkg) { + return "", false + } + return filepath.Join(sourceRoot, "lib", filepath.FromSlash(within)), true + } + if !strings.EqualFold(scheme, "file") { + return "", false // an unknown scheme is not a path to guess at + } + } + + // A path: either as the build machine spelled it, or the same file in this + // checkout. Both are checked against the vendor markers, since the resolved + // path is what establishes whose code it is. + if isFlutterVendorSource(key) || isFlutterVendorSource(resolved) { + return "", false + } + if _, err := os.Stat(resolved); err == nil { + return resolved, true + } + if alt := resolveFlutterSourceFallback(resolved, byBase); alt != "" { + return alt, true + } + return "", false +} + +// flutterURIScheme returns the scheme of a Dart script URI, or "" when the name +// is a filesystem path. A Windows drive letter is a path, not a scheme. +func flutterURIScheme(name string) string { + i := strings.Index(name, ":") + if i <= 1 { + return "" + } + for j := 0; j < i; j++ { + c := name[j] + if !(c >= 'a' && c <= 'z') && !(c >= 'A' && c <= 'Z') && !(c >= '0' && c <= '9') && c != '-' && c != '+' && c != '.' { + return "" + } + } + return name[:i] +} + +// flutterURIPath strips a scheme so the extension can be read off either form. +func flutterURIPath(name string) string { + if scheme := flutterURIScheme(name); scheme != "" { + return strings.TrimPrefix(name[len(scheme)+1:], "//") + } + return name +} + +// flutterAppPackage reads the package name out of the project's pubspec.yaml, +// which is what its own "package:" URIs are keyed by. Returns "" when there is no +// readable pubspec, in which case package URIs are left unresolved rather than +// guessed at. +func flutterAppPackage(sourceRoot string) string { + if sourceRoot == "" { + return "" + } + raw, err := os.ReadFile(filepath.Join(sourceRoot, "pubspec.yaml")) + if err != nil { + return "" + } + + for line := range strings.SplitSeq(string(raw), "\n") { + line = strings.TrimSuffix(line, "\r") + // Top level only: a "name:" indented under dependencies belongs to + // something else. + if strings.HasPrefix(line, " ") || strings.HasPrefix(line, "\t") { + continue + } + rest, ok := strings.CutPrefix(line, "name:") + if !ok { + continue + } + name := strings.TrimSpace(rest) + name = strings.Trim(name, `"'`) + if i := strings.Index(name, "#"); i >= 0 { + name = strings.TrimSpace(name[:i]) + } + return name + } + return "" +} + +// indexDartFilesByBase maps basename → paths under root, for recovering a file +// whose recorded path belongs to the machine that built it. Empty when root is +// blank or unreadable. +func indexDartFilesByBase(root string) map[string][]string { + out := make(map[string][]string) + if root == "" { + return out + } + info, err := os.Stat(root) + if err != nil || !info.IsDir() { + return out + } + _ = filepath.WalkDir(root, func(p string, d fs.DirEntry, walkErr error) error { + if walkErr != nil { + return nil + } + if d.IsDir() { + switch d.Name() { + case ".dart_tool", "build", ".git", ".pub-cache": + return filepath.SkipDir + } + return nil + } + if !flutterSourceExtensions[strings.ToLower(filepath.Ext(d.Name()))] { + return nil + } + if isFlutterVendorSource(p) { + return nil + } + out[d.Name()] = append(out[d.Name()], p) + return nil + }) + return out +} + +// resolveFlutterSourceFallback picks a file from the local checkout for a +// recorded path that is not readable here. A unique basename match is taken; when +// several files share the name, only one whose path ends with the recorded path +// is, since anything else would be a coin flip between same-named files. +func resolveFlutterSourceFallback(recorded string, byBase map[string][]string) string { + base := filepath.Base(recorded) + candidates := byBase[base] + if len(candidates) == 0 { + return "" + } + if len(candidates) == 1 { + return candidates[0] + } + suffix := filepath.ToSlash(recorded) + for _, c := range candidates { + if strings.HasSuffix(filepath.ToSlash(c), suffix) { + return c + } + } + return "" +} + +// flutterSourceKeyBeside returns the storage key for the source bundle that sits +// next to a .dartmap key. +func flutterSourceKeyBeside(dartmapKey string) string { + dir := path.Dir(filepath.ToSlash(dartmapKey)) + if dir == "." || dir == "" { + return flutterSourceBundleName + } + return dir + "/" + flutterSourceBundleName +} diff --git a/cmd/symbols/flutter_sources_test.go b/cmd/symbols/flutter_sources_test.go new file mode 100644 index 00000000..5c905cac --- /dev/null +++ b/cmd/symbols/flutter_sources_test.go @@ -0,0 +1,171 @@ +package symbols + +import ( + "os" + "path/filepath" + "testing" + + "github.com/launchdarkly/ldcli/internal/symbols/flutter" + "github.com/launchdarkly/ldcli/internal/symbols/srcbundle" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// flutterProject writes a minimal Flutter project (pubspec + lib file) and +// returns its root. +func flutterProject(t *testing.T, pkg, libRelPath, body string) string { + t.Helper() + root := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(root, "pubspec.yaml"), + []byte("name: "+pkg+"\ndescription: test\n"), 0o644)) + full := filepath.Join(root, "lib", filepath.FromSlash(libRelPath)) + require.NoError(t, os.MkdirAll(filepath.Dir(full), 0o755)) + require.NoError(t, os.WriteFile(full, []byte(body), 0o644)) + return root +} + +// The app's own "package:" URIs resolve through pubspec.yaml to lib/, and the +// bundle is keyed by the URI the .dartmap stores rather than by a path. +func TestBuildFlutterSourceBundleResolvesAppPackageURIs(t *testing.T) { + root := flutterProject(t, "my_app", "src/cart.dart", "class Cart {}\n") + + images := []flutter.Image{{Sources: map[string]string{ + "package:my_app/src/cart.dart": "package:my_app/src/cart.dart", + }}} + + raw, n, err := buildFlutterSourceBundle(images, root) + require.NoError(t, err) + require.NotNil(t, raw) + assert.Equal(t, 1, n) + + bundle, err := srcbundle.Open(raw) + require.NoError(t, err) + got, ok := bundle.File("package:my_app/src/cart.dart") + require.True(t, ok, "the app's own package URI should be packed under that URI") + assert.Equal(t, "class Cart {}\n", string(got)) +} + +// The SDK and dependency URIs must be left out entirely — never satisfied by a +// same-named file in the project, which would show the wrong code behind a real +// frame. +func TestBuildFlutterSourceBundleExcludesSDKAndDependencyURIs(t *testing.T) { + root := flutterProject(t, "my_app", "ink_well.dart", "// the app's own file\n") + + images := []flutter.Image{{Sources: map[string]string{ + "package:flutter/src/material/ink_well.dart": "package:flutter/src/material/ink_well.dart", + "package:http/src/client.dart": "package:http/src/client.dart", + "dart:async": "dart:async", + "org-dartlang-sdk:///sdk/lib/core/list.dart": "org-dartlang-sdk:///sdk/lib/core/list.dart", + }}} + + raw, n, err := buildFlutterSourceBundle(images, root) + require.Nil(t, raw, "nothing in this image belongs to the app") + require.NoError(t, err) + assert.Equal(t, 0, n) +} + +// A file:// URI names a real path once its scheme is stripped, which is what the +// DWARF reader hands over. +func TestBuildFlutterSourceBundleReadsFileURIs(t *testing.T) { + root := t.TempDir() + src := filepath.Join(root, "lib", "main.dart") + require.NoError(t, os.MkdirAll(filepath.Dir(src), 0o755)) + require.NoError(t, os.WriteFile(src, []byte("void main() {}\n"), 0o644)) + + key := "file://" + filepath.ToSlash(src) + images := []flutter.Image{{Sources: map[string]string{key: src}}} + + raw, n, err := buildFlutterSourceBundle(images, "") + require.NoError(t, err) + require.NotNil(t, raw) + assert.Equal(t, 1, n) + + bundle, err := srcbundle.Open(raw) + require.NoError(t, err) + _, ok := bundle.File(key) + assert.True(t, ok, "the bundle is keyed by the URI the map stores") +} + +// A path recorded on the build machine is recovered from the local checkout. +func TestBuildFlutterSourceBundleSourcePathFallback(t *testing.T) { + root := t.TempDir() + local := filepath.Join(root, "lib", "main.dart") + require.NoError(t, os.MkdirAll(filepath.Dir(local), 0o755)) + body := []byte("class Cart {}\n") + require.NoError(t, os.WriteFile(local, body, 0o644)) + + images := []flutter.Image{{Sources: map[string]string{ + "/ci/checkout/lib/main.dart": "/ci/checkout/lib/main.dart", + }}} + + raw, n, err := buildFlutterSourceBundle(images, root) + require.NoError(t, err) + require.NotNil(t, raw) + assert.Equal(t, 1, n) + + bundle, err := srcbundle.Open(raw) + require.NoError(t, err) + got, ok := bundle.File("/ci/checkout/lib/main.dart") + require.True(t, ok) + assert.Equal(t, body, got) +} + +// Without a readable pubspec there is no way to tell the app's package from a +// dependency's, so package URIs are left unresolved rather than guessed at. +func TestBuildFlutterSourceBundleWithoutPubspecSkipsPackageURIs(t *testing.T) { + root := t.TempDir() + lib := filepath.Join(root, "lib", "main.dart") + require.NoError(t, os.MkdirAll(filepath.Dir(lib), 0o755)) + require.NoError(t, os.WriteFile(lib, []byte("void main() {}\n"), 0o644)) + + images := []flutter.Image{{Sources: map[string]string{ + "package:my_app/main.dart": "package:my_app/main.dart", + }}} + + raw, _, err := buildFlutterSourceBundle(images, root) + require.NoError(t, err) + assert.Nil(t, raw) +} + +func TestFlutterAppPackage(t *testing.T) { + root := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(root, "pubspec.yaml"), []byte( + "name: my_app # the app\ndescription: x\ndependencies:\n http:\n name: not_this\n", + ), 0o644)) + assert.Equal(t, "my_app", flutterAppPackage(root)) + + assert.Equal(t, "", flutterAppPackage(t.TempDir()), "no pubspec, no package name") + assert.Equal(t, "", flutterAppPackage("")) +} + +func TestFlutterURISchemeTreatsWindowsPathsAsPaths(t *testing.T) { + assert.Equal(t, "package", flutterURIScheme("package:my_app/main.dart")) + assert.Equal(t, "dart", flutterURIScheme("dart:async")) + assert.Equal(t, "file", flutterURIScheme("file:///a/b.dart")) + assert.Equal(t, "", flutterURIScheme(`C:\src\main.dart`)) + assert.Equal(t, "", flutterURIScheme("lib/main.dart")) +} + +func TestFlutterSourceKeyBeside(t *testing.T) { + assert.Equal(t, + "_sym/flutter/id/abc123/sources.srcbundle", + flutterSourceKeyBeside("_sym/flutter/id/abc123/app.dartmap"), + ) + assert.Equal(t, + "1.2.3/sources.srcbundle", + flutterSourceKeyBeside("1.2.3/app.android-arm64.dartmap"), + ) +} + +func TestIsFlutterVendorSource(t *testing.T) { + assert.True(t, isFlutterVendorSource("/Users/dev/.pub-cache/hosted/pub.dev/foo/lib/a.dart")) + assert.True(t, isFlutterVendorSource("/sdk/flutter/packages/flutter/lib/material.dart")) + assert.False(t, isFlutterVendorSource("/Users/dev/myapp/lib/main.dart")) +} + +// Every lane a map is stored on gets the bundle, because the backend reads it +// from whichever lane resolved the map, and each arch has its own Id lane. +func TestBuildFlutterMapsAttachesSourcesToEveryLane(t *testing.T) { + assert.Equal(t, "_sym/flutter/id/a/sources.srcbundle", flutterSourceKeyBeside(flutterIDKey("a"))) + assert.Equal(t, "9/sources.srcbundle", flutterSourceKeyBeside(flutterVersionKey("9", "android-arm64"))) +} diff --git a/cmd/symbols/flutter_upload.go b/cmd/symbols/flutter_upload.go index 636e392a..3fa64d3f 100644 --- a/cmd/symbols/flutter_upload.go +++ b/cmd/symbols/flutter_upload.go @@ -32,9 +32,9 @@ const ( flutterSymbolFileSuffix = ".symbols" ) -// flutterUpload is one .dartmap object to store at one key. A map is uploaded to -// the Id lane always, and to the Version lane too when --app-version is given -// (same bytes, two keys). +// flutterUpload is one object to store at one key — a .dartmap, or the optional +// sources.srcbundle that sits beside it. A map is uploaded to the Id lane always, +// and to the Version lane too when --app-version is given (same bytes, two keys). type flutterUpload struct { Data []byte Key string @@ -43,12 +43,13 @@ type flutterUpload struct { // uploadFlutterSymbols discovers app.*.symbols files under path, compiles each // to a .dartmap, and uploads it to the Id lane (and the Version lane when -// appVersion is set). +// appVersion is set). With includeSources it also packs the project's .dart +// files into a sources.srcbundle beside each map. // // With skipExisting only the Id-lane copy can be skipped: a rebuild under the same // --app-version must still replace what that version resolves to. -func uploadFlutterSymbols(apiKey, projectID, path, appVersion, backendURL string, skipExisting bool) error { - uploads, err := buildFlutterMaps(path, appVersion) +func uploadFlutterSymbols(apiKey, projectID, path, appVersion, backendURL string, includeSources bool, sourceRoot string, skipExisting bool) error { + uploads, err := buildFlutterMaps(path, appVersion, includeSources, sourceRoot) if err != nil { return err } @@ -58,8 +59,9 @@ func uploadFlutterSymbols(apiKey, projectID, path, appVersion, backendURL string keys[i] = u.Key } - // No digests: every key here is either the dartmap's own build id or a Version - // Lane copy, which the backend re-presigns so it can overwrite. + // No digests: every key here is either the dartmap's own build id, a Version + // Lane copy, or a source bundle that borrows that id — which the backend + // re-presigns so it can overwrite. uploadURLs, err := getSymbolUploadUrls(apiKey, projectID, keys, nil, backendURL, skipExisting) if err != nil { return fmt.Errorf("failed to get upload URLs: %w", err) @@ -79,7 +81,7 @@ func uploadFlutterSymbols(apiKey, projectID, path, appVersion, backendURL string // Nothing here carries a digest, so unlike the Apple uploader this can wait // until a map is known to be going, and skip the work for one that isn't. if err := uploadBytes(compressBody(u.Data), uploadURLs[i], u.Label); err != nil { - return fmt.Errorf("failed to upload symbol map %s: %w", u.Label, err) + return fmt.Errorf("failed to upload %s: %w", u.Label, err) } } @@ -91,7 +93,10 @@ func uploadFlutterSymbols(apiKey, projectID, path, appVersion, backendURL string // returns the objects to store, deduplicating by symbols_id (the same build can // be discovered more than once). Each map yields an Id-lane upload, plus a // Version-lane upload when appVersion and a platform token are both available. -func buildFlutterMaps(path, appVersion string) ([]flutterUpload, error) { +// With includeSources one sources.srcbundle is attached beside every distinct +// storage prefix the maps occupy, since symbolication reads it from the lane the +// map came from. +func buildFlutterMaps(path, appVersion string, includeSources bool, sourceRoot string) ([]flutterUpload, error) { files, err := findFlutterSymbolFiles(path) if err != nil { return nil, fmt.Errorf("failed to find Flutter symbol files: %w", err) @@ -101,6 +106,7 @@ func buildFlutterMaps(path, appVersion string) ([]flutterUpload, error) { } var uploads []flutterUpload + var images []flutter.Image seenID := make(map[string]bool) seenVersionKey := make(map[string]bool) var noBuildID []string @@ -110,6 +116,7 @@ func buildFlutterMaps(path, appVersion string) ([]flutterUpload, error) { if err != nil { return nil, fmt.Errorf("failed to process %s: %w", file, err) } + images = append(images, img) var buf bytes.Buffer if err := img.Builder.Encode(&buf); err != nil { @@ -184,6 +191,40 @@ func buildFlutterMaps(path, appVersion string) ([]flutterUpload, error) { } return nil, fmt.Errorf("no Flutter symbol maps could be built from %s", path) } + + if includeSources { + sources, n, err := buildFlutterSourceBundle(images, sourceRoot) + if err != nil { + return nil, err + } + if sources == nil { + fmt.Printf("No project .dart sources could be read for --%s (--%s %q is not a Flutter project root, or its sources are not the ones this build was compiled from); continuing with symbol maps only\n", includeSourcesFlag, sourcePathFlag, sourceRoot) + return uploads, nil + } + + // Every lane gets its own copy. Symbolication reads the bundle from + // whichever lane resolved the map, and each arch has a lane of its own, so + // a single copy would leave crashes from the other arches without source. + srcKeys := make([]string, 0, len(uploads)) + seenSrc := make(map[string]bool) + for _, u := range uploads { + srcKey := flutterSourceKeyBeside(u.Key) + if seenSrc[srcKey] { + continue + } + seenSrc[srcKey] = true + srcKeys = append(srcKeys, srcKey) + } + for _, srcKey := range srcKeys { + uploads = append(uploads, flutterUpload{ + Data: sources, + Key: srcKey, + Label: fmt.Sprintf("%s (%d files)", flutterSourceBundleName, n), + }) + } + fmt.Printf("Built source bundle (%d files, %d bytes) for %d lane(s)\n", n, len(sources), len(srcKeys)) + } + return uploads, nil } diff --git a/cmd/symbols/generate.go b/cmd/symbols/generate.go index 64fc64a5..189f4889 100644 --- a/cmd/symbols/generate.go +++ b/cmd/symbols/generate.go @@ -82,7 +82,7 @@ func generateRunE() func(cmd *cobra.Command, args []string) error { // Flutter symbols compile to .dartmap maps keyed by build id (Id Lane), // plus a Version-lane copy when --app-version is set. if symbolType == typeFlutter { - return generateFlutterSymbols(path, viper.GetString(appVersionFlag), outputDir) + return generateFlutterSymbols(path, viper.GetString(appVersionFlag), outputDir, viper.GetBool(includeSourcesFlag), viper.GetString(sourcePathFlag)) } // An Android mapping compiles to the index symbolication reads, on the Id and @@ -128,8 +128,8 @@ func generateAppleDSYMs(path, outputDir string, includeSources bool) error { // generateFlutterSymbols compiles the discovered app.*.symbols to .dartmap // symbol maps and writes them under outputDir using the same storage keys // `symbols upload` would use (Id lane, plus Version lane when appVersion is set). -func generateFlutterSymbols(path, appVersion, outputDir string) error { - uploads, err := buildFlutterMaps(path, appVersion) +func generateFlutterSymbols(path, appVersion, outputDir string, includeSources bool, sourceRoot string) error { + uploads, err := buildFlutterMaps(path, appVersion, includeSources, sourceRoot) if err != nil { return err } @@ -235,10 +235,10 @@ func initGenerateFlags(cmd *cobra.Command) { cmd.Flags().String(appVersionFlag, "", "The current version of your deploy") _ = viper.BindPFlag(appVersionFlag, cmd.Flags().Lookup(appVersionFlag)) - cmd.Flags().Bool(includeSourcesFlag, false, fmt.Sprintf("Also generate a source bundle, for source context around native frames (%s and %s)", typeAppleDSYM, typeAndroid)) + cmd.Flags().Bool(includeSourcesFlag, false, fmt.Sprintf("Also generate a source bundle, for source context around native frames (%s, %s, and %s)", typeAppleDSYM, typeAndroid, typeFlutter)) _ = viper.BindPFlag(includeSourcesFlag, cmd.Flags().Lookup(includeSourcesFlag)) - cmd.Flags().String(sourcePathFlag, defaultPath, fmt.Sprintf("Directory to scan for .java/.kt sources when using --%s with --type %s", includeSourcesFlag, typeAndroid)) + cmd.Flags().String(sourcePathFlag, defaultPath, fmt.Sprintf("Directory to resolve your sources from when using --%s: the tree to scan for .java/.kt with --type %s, or your Flutter project root (the directory holding pubspec.yaml) with --type %s", includeSourcesFlag, typeAndroid, typeFlutter)) _ = viper.BindPFlag(sourcePathFlag, cmd.Flags().Lookup(sourcePathFlag)) cmd.Flags().String(symbolsIdFlag, "", "The symbols id (launchdarkly.symbols_id.htlhash) to key files by (Symbols Id Lane). If omitted, a *.symbolsid sidecar next to the bundle is used when present") diff --git a/cmd/symbols/upload.go b/cmd/symbols/upload.go index f5e8fd24..004c4007 100644 --- a/cmd/symbols/upload.go +++ b/cmd/symbols/upload.go @@ -246,7 +246,7 @@ func runE(client resources.Client) func(cmd *cobra.Command, args []string) error // Version-lane copy when --app-version is set. if symbolType == typeFlutter { fmt.Printf("Starting to upload %s symbols from %s\n", symbolType, path) - return uploadFlutterSymbols(viper.GetString(cliflags.AccessTokenFlag), projectResult.ID, path, appVersion, backendUrl, skipExisting) + return uploadFlutterSymbols(viper.GetString(cliflags.AccessTokenFlag), projectResult.ID, path, appVersion, backendUrl, viper.GetBool(includeSourcesFlag), viper.GetString(sourcePathFlag), skipExisting) } // Android takes a dedicated path as well: the R8 mapping is compiled into the @@ -745,12 +745,12 @@ func initFlags(cmd *cobra.Command) { cmd.Flags().String(backendUrlFlag, "", fmt.Sprintf("An optional backend url for self-hosted deployments. Defaults to the observability API of whichever instance --%s names (%s for the default)", cliflags.BaseURIFlag, defaultBackendUrl)) _ = viper.BindPFlag(backendUrlFlag, cmd.Flags().Lookup(backendUrlFlag)) - cmd.Flags().Bool(includeSourcesFlag, false, fmt.Sprintf("Also upload your source files so the errors page can show source context around native frames (%s and %s). Your source is stored in LaunchDarkly", typeAppleDSYM, typeAndroid)) + cmd.Flags().Bool(includeSourcesFlag, false, fmt.Sprintf("Also upload your source files so the errors page can show source context around native frames (%s, %s, and %s). Your source is stored in LaunchDarkly", typeAppleDSYM, typeAndroid, typeFlutter)) _ = viper.BindPFlag(includeSourcesFlag, cmd.Flags().Lookup(includeSourcesFlag)) cmd.Flags().Bool(noSkipExistingFlag, false, "Re-upload symbols even when LaunchDarkly already has them. By default a symbols-id file that is already stored is skipped, since its id is derived from its contents") _ = viper.BindPFlag(noSkipExistingFlag, cmd.Flags().Lookup(noSkipExistingFlag)) - cmd.Flags().String(sourcePathFlag, defaultPath, fmt.Sprintf("Directory to scan for .java/.kt sources when using --%s with --type %s", includeSourcesFlag, typeAndroid)) + cmd.Flags().String(sourcePathFlag, defaultPath, fmt.Sprintf("Directory to resolve your sources from when using --%s: the tree to scan for .java/.kt with --type %s, or your Flutter project root (the directory holding pubspec.yaml, which names the package your .dart files are compiled under) with --type %s", includeSourcesFlag, typeAndroid, typeFlutter)) _ = viper.BindPFlag(sourcePathFlag, cmd.Flags().Lookup(sourcePathFlag)) } diff --git a/internal/symbols/flutter/elf.go b/internal/symbols/flutter/elf.go index 2173c8d6..3aa77049 100644 --- a/internal/symbols/flutter/elf.go +++ b/internal/symbols/flutter/elf.go @@ -20,6 +20,7 @@ import ( "debug/elf" "encoding/binary" "encoding/hex" + "net/url" "path/filepath" "strings" @@ -42,6 +43,13 @@ type Image struct { // app..symbols filename — used for the Version-lane object name. Platform string Builder *dsymmap.Builder + // Sources maps each source path referenced by this image's DWARF — keyed by + // the exact string stored in the .dartmap, so a resolved frame's FileName is + // the lookup key — to its absolute path on this machine. It is only used to + // build the optional .srcbundle (`--include-sources`). Every path the DWARF + // mentions is included, SDK and pub-cache among them; selecting which of + // those may be uploaded is the bundler's job. + Sources map[string]string } // BuildFromELF opens a Flutter app..symbols ELF at path and returns @@ -67,7 +75,8 @@ func BuildFromELF(path string) (Image, error) { // Dart AOT addresses in the crash `virt` column are already snapshot-relative // and match the DWARF vaddr, so no rebasing is needed (TextVMAddr = 0). b := &dsymmap.Builder{} - if err := populate(d, b); err != nil { + sources := make(map[string]string) + if err := populate(d, b, sources); err != nil { return Image{}, err } @@ -75,6 +84,7 @@ func BuildFromELF(path string) (Image, error) { SymbolsID: symbolsID, Platform: platformFromFilename(path), Builder: b, + Sources: sources, }, nil } @@ -170,12 +180,15 @@ type scope struct { // populate walks the DWARF DIE tree into b: physical functions, their line // tables, and inlined-call chains. Mirrors apple.populate but on the standard -// library's debug/dwarf types. -func populate(d *dwarf.Data, b *dsymmap.Builder) error { +// library's debug/dwarf types. It also records every source path it encounters +// into sources (keyed exactly as stored in the map) so the caller can optionally +// bundle those files; pass nil to skip that. +func populate(d *dwarf.Data, b *dsymmap.Builder, sources map[string]string) error { r := d.Reader() var stack []scope var funcs []*dsymmap.Function var curFiles []*dwarf.LineFile + var curCompDir string top := func() scope { if len(stack) == 0 { @@ -205,7 +218,8 @@ func populate(d *dwarf.Data, b *dsymmap.Builder) error { switch ent.Tag { case dwarf.TagCompileUnit: curFiles = filesForCU(d, ent) - addLines(d, ent, b) + curCompDir, _ = ent.Val(dwarf.AttrCompDir).(string) + addLines(d, ent, b, sources, curCompDir) case dwarf.TagSubprogram: if fn := makeFunction(d, ent); fn != nil { @@ -217,7 +231,11 @@ func populate(d *dwarf.Data, b *dsymmap.Builder) error { case dwarf.TagInlinedSubroutine: depth := top().inlineDepth + 1 if fn := top().fn; fn != nil { - fn.Inlines = append(fn.Inlines, makeInlines(d, ent, depth, curFiles)...) + inlines := makeInlines(d, ent, depth, curFiles) + for i := range inlines { + recordSource(sources, inlines[i].CallFile, curCompDir) + } + fn.Inlines = append(fn.Inlines, inlines...) } push.inlineDepth = depth } @@ -233,6 +251,68 @@ func populate(d *dwarf.Data, b *dsymmap.Builder) error { return nil } +// recordSource notes that file (as spelled in the map) is referenced by this +// image, resolving where to read it from on this machine. +// +// Dart names a compilation unit by its script URI, not by a filesystem path, so +// what lands here is one of "package:my_app/main.dart", "dart:async", +// "org-dartlang-sdk:///...", "file:///abs/path.dart", or a plain path. Only the +// last two name something openable, and a file URI has to have its scheme +// stripped first. The rest keep the URI as their value: it is not a path, and +// the caller — which knows the project root and the app's package name — is the +// only one that can resolve or reject them. +func recordSource(sources map[string]string, file, compDir string) { + if sources == nil || file == "" { + return + } + if _, ok := sources[file]; ok { + return + } + sources[file] = resolveSourcePath(file, compDir) +} + +// resolveSourcePath returns where to read a DWARF file name from, or the name +// unchanged when it is a URI this cannot resolve. +func resolveSourcePath(file, compDir string) string { + if rest, ok := strings.CutPrefix(file, "file://"); ok { + // "file:///abs/path" leaves a leading slash, which is the path. A + // host-qualified file URI ("file://host/path") is not something a Dart + // build produces, so it is left to the caller to reject. + if decoded, err := url.PathUnescape(rest); err == nil { + rest = decoded + } + if strings.HasPrefix(rest, "/") { + return rest + } + return file + } + // Any other scheme ("dart:", "package:", "org-dartlang-sdk:") is not a path. + if hasURIScheme(file) { + return file + } + if !filepath.IsAbs(file) && compDir != "" { + return filepath.Join(compDir, file) + } + return file +} + +// hasURIScheme reports whether name opens with a URI scheme rather than being a +// filesystem path. A Windows drive letter ("C:\src\main.dart") is not a scheme, +// so a single-character prefix does not count. +func hasURIScheme(name string) bool { + i := strings.Index(name, ":") + if i <= 1 { + return false + } + for j := 0; j < i; j++ { + c := name[j] + if !(c >= 'a' && c <= 'z') && !(c >= 'A' && c <= 'Z') && !(c >= '0' && c <= '9') && c != '-' && c != '+' && c != '.' { + return false + } + } + return true +} + func makeFunction(d *dwarf.Data, ent *dwarf.Entry) *dsymmap.Function { lo, hi, ok := spanOf(d, ent) if !ok { @@ -301,7 +381,7 @@ func callSite(ent *dwarf.Entry, files []*dwarf.LineFile) (string, uint32) { return file, line } -func addLines(d *dwarf.Data, cu *dwarf.Entry, b *dsymmap.Builder) { +func addLines(d *dwarf.Data, cu *dwarf.Entry, b *dsymmap.Builder, sources map[string]string, compDir string) { lr, err := d.LineReader(cu) if err != nil || lr == nil { return @@ -321,6 +401,7 @@ func addLines(d *dwarf.Data, cu *dwarf.Entry, b *dsymmap.Builder) { if le.File != nil { file = le.File.Name } + recordSource(sources, file, compDir) b.Lines = append(b.Lines, dsymmap.LineRow{Addr: le.Address, File: file, Line: uint32(le.Line)}) } } diff --git a/internal/symbols/flutter/elf_test.go b/internal/symbols/flutter/elf_test.go index 87fac629..28c32fe1 100644 --- a/internal/symbols/flutter/elf_test.go +++ b/internal/symbols/flutter/elf_test.go @@ -56,6 +56,28 @@ func TestBuildIDFromNotes(t *testing.T) { assert.Equal(t, "0f8a1b2c3d4e5f60", buildIDFromNotes(note(be, "GNU", ntGNUBuildID, id), be)) } +// Dart names a compilation unit by its script URI, so only the plain and file:// +// forms resolve to something readable; the rest keep the URI for the bundler to +// resolve or reject. +func TestResolveSourcePath(t *testing.T) { + assert.Equal(t, "/abs/lib/main.dart", resolveSourcePath("file:///abs/lib/main.dart", "")) + assert.Equal(t, "/abs/my app/main.dart", resolveSourcePath("file:///abs/my%20app/main.dart", "")) + assert.Equal(t, "/build/dir/lib/main.dart", resolveSourcePath("../lib/main.dart", "/build/dir/tool")) + assert.Equal(t, "/already/abs.dart", resolveSourcePath("/already/abs.dart", "/build/dir")) + // Non-file schemes are not paths: they survive untouched. + assert.Equal(t, "package:my_app/main.dart", resolveSourcePath("package:my_app/main.dart", "/build/dir")) + assert.Equal(t, "dart:async", resolveSourcePath("dart:async", "/build/dir")) + assert.Equal(t, "org-dartlang-sdk:///sdk/lib/core/list.dart", resolveSourcePath("org-dartlang-sdk:///sdk/lib/core/list.dart", "/build/dir")) +} + +func TestHasURISchemeTreatsWindowsDriveAsPath(t *testing.T) { + assert.True(t, hasURIScheme("package:my_app/main.dart")) + assert.True(t, hasURIScheme("dart:async")) + assert.False(t, hasURIScheme(`C:\src\main.dart`)) + assert.False(t, hasURIScheme("lib/main.dart")) + assert.False(t, hasURIScheme("main.dart")) +} + func TestPlatformFromFilename(t *testing.T) { assert.Equal(t, "android-arm64", platformFromFilename("app.android-arm64.symbols")) assert.Equal(t, "ios-arm64", platformFromFilename("build/symbols/app.ios-arm64.symbols"))