From b1d8f8c8506f351e1ee9ba1ac7952076892eab88 Mon Sep 17 00:00:00 2001 From: Andrey Belonogov Date: Fri, 7 Aug 2026 18:04:20 -0700 Subject: [PATCH 1/2] Add Flutter --include-sources for Dart stack context. Pack DWARF-referenced .dart files into sources.srcbundle beside each .dartmap so symbolicated Flutter frames can show code snippets, matching the Apple/Android upload path. Co-authored-by: Cursor --- cmd/symbols/flutter_sources.go | 177 ++++++++++++++++++++++++++++ cmd/symbols/flutter_sources_test.go | 81 +++++++++++++ cmd/symbols/flutter_upload.go | 50 ++++++-- cmd/symbols/generate.go | 10 +- cmd/symbols/upload.go | 6 +- internal/symbols/flutter/elf.go | 47 +++++++- 6 files changed, 348 insertions(+), 23 deletions(-) create mode 100644 cmd/symbols/flutter_sources.go create mode 100644 cmd/symbols/flutter_sources_test.go diff --git a/cmd/symbols/flutter_sources.go b/cmd/symbols/flutter_sources.go new file mode 100644 index 00000000..9f28c041 --- /dev/null +++ b/cmd/symbols/flutter_sources.go @@ -0,0 +1,177 @@ +package symbols + +import ( + "bytes" + "fmt" + "io/fs" + "os" + "path/filepath" + "strings" + + "github.com/launchdarkly/ldcli/internal/symbols/flutter" + "github.com/launchdarkly/ldcli/internal/symbols/srcbundle" +) + +// 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, +} + +// flutterVendorPathMarkers appear mid-path in Flutter/Dart SDK and pub-cache +// trees. Filtering by path (not readability) keeps SDK sources out of uploads +// the way Apple filters Xcode headers. +var flutterVendorPathMarkers = []string{ + "/.pub-cache/", + "/pub-cache/", + "/flutter/packages/flutter/", + "/flutter/bin/cache/", + "/flutter/packages/flutter_test/", + "/flutter/packages/flutter_driver/", + "/flutter/packages/flutter_localizations/", + "/flutter/packages/flutter_web_plugins/", + "/third_party/dart/", + "/hosted/pub.dev/", + "/hosted/pub.dartlang.org/", +} + +// isFlutterVendorSource reports whether path belongs to the Flutter/Dart SDK or +// pub cache rather than to the project being uploaded. +func isFlutterVendorSource(path string) bool { + lower := strings.ToLower(filepath.ToSlash(path)) + for _, marker := range flutterVendorPathMarkers { + if strings.Contains(lower, marker) { + return true + } + } + return false +} + +// buildFlutterSourceBundle packs the project's .dart sources referenced by one +// (or more) .symbols images' DWARF into a .srcbundle. Keys are the exact DWARF +// strings stored in the .dartmap, so a resolved frame's FileName is the lookup +// key. SDK and pub-cache paths are excluded; files that aren't on this machine +// are skipped (or recovered from sourceRoot when given). Returns nil when +// nothing local was found, so the caller can skip the upload. +func buildFlutterSourceBundle(images []flutter.Image, sourceRoot string) ([]byte, int, error) { + // Merge DWARF paths across arches: one release's sources are identical, and + // the backend reads the bundle from whichever lane resolved the map. + merged := make(map[string]string) + for _, img := range images { + for key, abs := range img.Sources { + if _, ok := merged[key]; !ok { + merged[key] = abs + } + } + } + + byBase := indexDartFilesByBase(sourceRoot) + + b := &srcbundle.Builder{} + total := 0 + for key, abs := range merged { + if !flutterSourceExtensions[strings.ToLower(filepath.Ext(key))] { + continue + } + if isFlutterVendorSource(key) || isFlutterVendorSource(abs) { + continue + } + data, err := os.ReadFile(abs) + if err != nil { + // DWARF recorded a build-machine path that isn't here; try the same + // basename under --source-path (typically the project root). + if alt := resolveFlutterSourceFallback(key, byBase); alt != "" { + data, err = os.ReadFile(alt) + } + } + if err != nil { + continue + } + 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 +} + +// indexDartFilesByBase maps basename → absolute paths under root. Empty when +// root is blank or unreadable; collisions keep every candidate so the caller can +// prefer a unique match. +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() { + name := d.Name() + if name == ".dart_tool" || name == "build" || name == ".git" || name == ".pub-cache" { + return filepath.SkipDir + } + return nil + } + if !flutterSourceExtensions[strings.ToLower(filepath.Ext(d.Name()))] { + return nil + } + if isFlutterVendorSource(p) { + return nil + } + base := d.Name() + out[base] = append(out[base], p) + return nil + }) + return out +} + +// resolveFlutterSourceFallback picks a --source-path file for a DWARF key whose +// recorded absolute path isn't readable here. Prefer a unique basename match; +// when several share the name, prefer one whose path ends with the DWARF key +// (handles package-relative keys like lib/main.dart). +func resolveFlutterSourceFallback(key string, byBase map[string][]string) string { + base := filepath.Base(key) + cands := byBase[base] + if len(cands) == 0 { + return "" + } + if len(cands) == 1 { + return cands[0] + } + slashKey := filepath.ToSlash(key) + for _, c := range cands { + if strings.HasSuffix(filepath.ToSlash(c), slashKey) { + return c + } + } + return "" +} + +// flutterSourceKeyBeside returns the storage key for a source bundle that sits +// next to a .dartmap key (same directory, sources.srcbundle name). +func flutterSourceKeyBeside(dartmapKey string) string { + dir := filepath.ToSlash(filepath.Dir(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..f18be8d1 --- /dev/null +++ b/cmd/symbols/flutter_sources_test.go @@ -0,0 +1,81 @@ +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" +) + +func TestBuildFlutterSourceBundleFromDWARFPaths(t *testing.T) { + root := t.TempDir() + mainPath := filepath.Join(root, "lib", "main.dart") + require.NoError(t, os.MkdirAll(filepath.Dir(mainPath), 0o755)) + require.NoError(t, os.WriteFile(mainPath, []byte("void main() {}\n"), 0o644)) + + images := []flutter.Image{{ + Sources: map[string]string{ + mainPath: mainPath, + "/Users/dev/.pub-cache/hosted/pub.dev/http-1.0.0/lib/http.dart": "/Users/dev/.pub-cache/hosted/pub.dev/http-1.0.0/lib/http.dart", + }, + }} + + 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(mainPath) + assert.True(t, ok, "project dart file should be in the bundle") + _, ok = bundle.File("/Users/dev/.pub-cache/hosted/pub.dev/http-1.0.0/lib/http.dart") + assert.False(t, ok, "pub-cache paths must be excluded") +} + +func TestBuildFlutterSourceBundleSourcePathFallback(t *testing.T) { + root := t.TempDir() + mainPath := filepath.Join(root, "lib", "main.dart") + require.NoError(t, os.MkdirAll(filepath.Dir(mainPath), 0o755)) + body := []byte("class Cart {}\n") + require.NoError(t, os.WriteFile(mainPath, body, 0o644)) + + // DWARF recorded a build-machine absolute path that isn't here. + 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) +} + +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")) +} diff --git a/cmd/symbols/flutter_upload.go b/cmd/symbols/flutter_upload.go index 636e392a..4f736ba3 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) @@ -91,7 +93,9 @@ 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 a single sources.srcbundle is attached beside each unique +// storage prefix the maps occupy. +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 +105,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 +115,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 +190,32 @@ 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 .dart sources found to upload (DWARF paths unreadable and --source-path %q empty); continuing with symbol maps only\n", sourceRoot) + } else { + seenSrc := make(map[string]bool) + for _, u := range uploads { + srcKey := flutterSourceKeyBeside(u.Key) + if seenSrc[srcKey] { + continue + } + seenSrc[srcKey] = true + 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)\n", n, len(sources)) + } + } + return uploads, nil } diff --git a/cmd/symbols/generate.go b/cmd/symbols/generate.go index 64fc64a5..ba98fd49 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 project sources from when using --%s (.java/.kt for --type %s; .dart for --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..c468546f 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 project sources from when using --%s (.%s/.%s for --type %s; .dart for --type %s, also used when DWARF paths from the build machine are unreadable here)", includeSourcesFlag, "java", "kt", 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..baf5f854 100644 --- a/internal/symbols/flutter/elf.go +++ b/internal/symbols/flutter/elf.go @@ -42,6 +42,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 +74,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 +83,7 @@ func BuildFromELF(path string) (Image, error) { SymbolsID: symbolsID, Platform: platformFromFilename(path), Builder: b, + Sources: sources, }, nil } @@ -170,12 +179,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 +217,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 +230,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 +250,23 @@ 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. DWARF paths are +// usually absolute; relative ones are resolved against the CU's DW_AT_comp_dir. +func recordSource(sources map[string]string, file, compDir string) { + if sources == nil || file == "" { + return + } + if _, ok := sources[file]; ok { + return + } + abs := file + if !filepath.IsAbs(abs) && compDir != "" { + abs = filepath.Join(compDir, abs) + } + sources[file] = abs +} + func makeFunction(d *dwarf.Data, ent *dwarf.Entry) *dsymmap.Function { lo, hi, ok := spanOf(d, ent) if !ok { @@ -301,7 +335,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 +355,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)}) } } From 11f343610b9d58f1bec06ad751de117a8e340460 Mon Sep 17 00:00:00 2001 From: Andrey Belonogov Date: Fri, 7 Aug 2026 18:18:11 -0700 Subject: [PATCH 2/2] Resolve Dart script URIs when bundling Flutter sources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dart names a compilation unit by its script URI, so what a .dartmap stores as a frame's file is usually "package:my_app/main.dart", "dart:async", or "org-dartlang-sdk:///...", not a path. os.ReadFile cannot open any of those, so --include-sources shipped an empty bundle for a real build, and the basename fallback would answer "package:flutter/src/material/ink_well.dart" with whatever local file shared its name — the wrong code behind a real frame. Package URIs now resolve through the project's pubspec name to lib/, file URIs have their scheme stripped, and the SDK and every dependency are excluded rather than guessed at. Bundle keys stay the URI the map spells, which is what the backend looks a frame up by. Co-authored-by: Cursor --- cmd/symbols/flutter_sources.go | 233 +++++++++++++++++++++------ cmd/symbols/flutter_sources_test.go | 136 +++++++++++++--- cmd/symbols/flutter_upload.go | 45 +++--- cmd/symbols/generate.go | 2 +- cmd/symbols/upload.go | 2 +- internal/symbols/flutter/elf.go | 58 ++++++- internal/symbols/flutter/elf_test.go | 22 +++ 7 files changed, 397 insertions(+), 101 deletions(-) diff --git a/cmd/symbols/flutter_sources.go b/cmd/symbols/flutter_sources.go index 9f28c041..919cfaea 100644 --- a/cmd/symbols/flutter_sources.go +++ b/cmd/symbols/flutter_sources.go @@ -5,13 +5,29 @@ import ( "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. @@ -22,27 +38,35 @@ 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. Filtering by path (not readability) keeps SDK sources out of uploads -// the way Apple filters Xcode headers. +// trees, for the builds whose DWARF records real paths instead of package URIs. var flutterVendorPathMarkers = []string{ "/.pub-cache/", "/pub-cache/", "/flutter/packages/flutter/", - "/flutter/bin/cache/", "/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 path belongs to the Flutter/Dart SDK or -// pub cache rather than to the project being uploaded. -func isFlutterVendorSource(path string) bool { - lower := strings.ToLower(filepath.ToSlash(path)) +// 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 @@ -51,45 +75,52 @@ func isFlutterVendorSource(path string) bool { return false } -// buildFlutterSourceBundle packs the project's .dart sources referenced by one -// (or more) .symbols images' DWARF into a .srcbundle. Keys are the exact DWARF -// strings stored in the .dartmap, so a resolved frame's FileName is the lookup -// key. SDK and pub-cache paths are excluded; files that aren't on this machine -// are skipped (or recovered from sourceRoot when given). Returns nil when -// nothing local was found, so the caller can skip the upload. +// 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 DWARF paths across arches: one release's sources are identical, and - // the backend reads the bundle from whichever lane resolved the map. + // 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, abs := range img.Sources { + for key, resolved := range img.Sources { if _, ok := merged[key]; !ok { - merged[key] = abs + 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, abs := range merged { - if !flutterSourceExtensions[strings.ToLower(filepath.Ext(key))] { + for _, key := range keys { + if !flutterSourceExtensions[strings.ToLower(path.Ext(flutterURIPath(key)))] { continue } - if isFlutterVendorSource(key) || isFlutterVendorSource(abs) { + local, ok := flutterSourceFile(key, merged[key], appPkg, sourceRoot, byBase) + if !ok { continue } - data, err := os.ReadFile(abs) - if err != nil { - // DWARF recorded a build-machine path that isn't here; try the same - // basename under --source-path (typically the project root). - if alt := resolveFlutterSourceFallback(key, byBase); alt != "" { - data, err = os.ReadFile(alt) - } - } + data, err := os.ReadFile(local) if err != nil { - continue + continue // built elsewhere: the backend renders the frame without source } if len(data) > maxSourceFileBytes || total+len(data) > maxSourceBundleBytes { continue @@ -108,9 +139,108 @@ func buildFlutterSourceBundle(images []flutter.Image, sourceRoot string) ([]byte return buf.Bytes(), b.Len(), nil } -// indexDartFilesByBase maps basename → absolute paths under root. Empty when -// root is blank or unreadable; collisions keep every candidate so the caller can -// prefer a unique match. +// 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 == "" { @@ -125,8 +255,8 @@ func indexDartFilesByBase(root string) map[string][]string { return nil } if d.IsDir() { - name := d.Name() - if name == ".dart_tool" || name == "build" || name == ".git" || name == ".pub-cache" { + switch d.Name() { + case ".dart_tool", "build", ".git", ".pub-cache": return filepath.SkipDir } return nil @@ -137,39 +267,38 @@ func indexDartFilesByBase(root string) map[string][]string { if isFlutterVendorSource(p) { return nil } - base := d.Name() - out[base] = append(out[base], p) + out[d.Name()] = append(out[d.Name()], p) return nil }) return out } -// resolveFlutterSourceFallback picks a --source-path file for a DWARF key whose -// recorded absolute path isn't readable here. Prefer a unique basename match; -// when several share the name, prefer one whose path ends with the DWARF key -// (handles package-relative keys like lib/main.dart). -func resolveFlutterSourceFallback(key string, byBase map[string][]string) string { - base := filepath.Base(key) - cands := byBase[base] - if len(cands) == 0 { +// 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(cands) == 1 { - return cands[0] + if len(candidates) == 1 { + return candidates[0] } - slashKey := filepath.ToSlash(key) - for _, c := range cands { - if strings.HasSuffix(filepath.ToSlash(c), slashKey) { + suffix := filepath.ToSlash(recorded) + for _, c := range candidates { + if strings.HasSuffix(filepath.ToSlash(c), suffix) { return c } } return "" } -// flutterSourceKeyBeside returns the storage key for a source bundle that sits -// next to a .dartmap key (same directory, sources.srcbundle name). +// flutterSourceKeyBeside returns the storage key for the source bundle that sits +// next to a .dartmap key. func flutterSourceKeyBeside(dartmapKey string) string { - dir := filepath.ToSlash(filepath.Dir(dartmapKey)) + dir := path.Dir(filepath.ToSlash(dartmapKey)) if dir == "." || dir == "" { return flutterSourceBundleName } diff --git a/cmd/symbols/flutter_sources_test.go b/cmd/symbols/flutter_sources_test.go index f18be8d1..5c905cac 100644 --- a/cmd/symbols/flutter_sources_test.go +++ b/cmd/symbols/flutter_sources_test.go @@ -11,18 +11,69 @@ import ( "github.com/stretchr/testify/require" ) -func TestBuildFlutterSourceBundleFromDWARFPaths(t *testing.T) { +// 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() - mainPath := filepath.Join(root, "lib", "main.dart") - require.NoError(t, os.MkdirAll(filepath.Dir(mainPath), 0o755)) - require.NoError(t, os.WriteFile(mainPath, []byte("void main() {}\n"), 0o644)) + 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)) - images := []flutter.Image{{ - Sources: map[string]string{ - mainPath: mainPath, - "/Users/dev/.pub-cache/hosted/pub.dev/http-1.0.0/lib/http.dart": "/Users/dev/.pub-cache/hosted/pub.dev/http-1.0.0/lib/http.dart", - }, - }} + key := "file://" + filepath.ToSlash(src) + images := []flutter.Image{{Sources: map[string]string{key: src}}} raw, n, err := buildFlutterSourceBundle(images, "") require.NoError(t, err) @@ -31,25 +82,21 @@ func TestBuildFlutterSourceBundleFromDWARFPaths(t *testing.T) { bundle, err := srcbundle.Open(raw) require.NoError(t, err) - _, ok := bundle.File(mainPath) - assert.True(t, ok, "project dart file should be in the bundle") - _, ok = bundle.File("/Users/dev/.pub-cache/hosted/pub.dev/http-1.0.0/lib/http.dart") - assert.False(t, ok, "pub-cache paths must be excluded") + _, 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() - mainPath := filepath.Join(root, "lib", "main.dart") - require.NoError(t, os.MkdirAll(filepath.Dir(mainPath), 0o755)) + 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(mainPath, body, 0o644)) + require.NoError(t, os.WriteFile(local, body, 0o644)) - // DWARF recorded a build-machine absolute path that isn't here. - images := []flutter.Image{{ - Sources: map[string]string{ - "/ci/checkout/lib/main.dart": "/ci/checkout/lib/main.dart", - }, - }} + 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) @@ -63,6 +110,42 @@ func TestBuildFlutterSourceBundleSourcePathFallback(t *testing.T) { 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", @@ -79,3 +162,10 @@ func TestIsFlutterVendorSource(t *testing.T) { 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 4f736ba3..3fa64d3f 100644 --- a/cmd/symbols/flutter_upload.go +++ b/cmd/symbols/flutter_upload.go @@ -81,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) } } @@ -93,8 +93,9 @@ 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. -// With includeSources a single sources.srcbundle is attached beside each unique -// storage prefix the maps occupy. +// 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 { @@ -197,23 +198,31 @@ func buildFlutterMaps(path, appVersion string, includeSources bool, sourceRoot s return nil, err } if sources == nil { - fmt.Printf("No .dart sources found to upload (DWARF paths unreadable and --source-path %q empty); continuing with symbol maps only\n", sourceRoot) - } else { - seenSrc := make(map[string]bool) - for _, u := range uploads { - srcKey := flutterSourceKeyBeside(u.Key) - if seenSrc[srcKey] { - continue - } - seenSrc[srcKey] = true - uploads = append(uploads, flutterUpload{ - Data: sources, - Key: srcKey, - Label: fmt.Sprintf("%s (%d files)", flutterSourceBundleName, n), - }) + 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 } - fmt.Printf("Built source bundle (%d files, %d bytes)\n", n, len(sources)) + 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 ba98fd49..189f4889 100644 --- a/cmd/symbols/generate.go +++ b/cmd/symbols/generate.go @@ -238,7 +238,7 @@ func initGenerateFlags(cmd *cobra.Command) { 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 resolve project sources from when using --%s (.java/.kt for --type %s; .dart for --type %s)", includeSourcesFlag, typeAndroid, typeFlutter)) + 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 c468546f..004c4007 100644 --- a/cmd/symbols/upload.go +++ b/cmd/symbols/upload.go @@ -751,6 +751,6 @@ func initFlags(cmd *cobra.Command) { 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 resolve project sources from when using --%s (.%s/.%s for --type %s; .dart for --type %s, also used when DWARF paths from the build machine are unreadable here)", includeSourcesFlag, "java", "kt", typeAndroid, typeFlutter)) + 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 baf5f854..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" @@ -251,8 +252,15 @@ func populate(d *dwarf.Data, b *dsymmap.Builder, sources map[string]string) erro } // recordSource notes that file (as spelled in the map) is referenced by this -// image, resolving where to read it from on this machine. DWARF paths are -// usually absolute; relative ones are resolved against the CU's DW_AT_comp_dir. +// 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 @@ -260,11 +268,49 @@ func recordSource(sources map[string]string, file, compDir string) { if _, ok := sources[file]; ok { return } - abs := file - if !filepath.IsAbs(abs) && compDir != "" { - abs = filepath.Join(compDir, abs) + 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 + } } - sources[file] = abs + return true } func makeFunction(d *dwarf.Data, ent *dwarf.Entry) *dsymmap.Function { 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"))