-
Notifications
You must be signed in to change notification settings - Fork 14
Add Flutter --include-sources for Dart stack context #780
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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/<symbolsID>/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 | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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")) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Source bundles lack upload digestsMedium Severity
Reviewed by Cursor Bugbot for commit b1d8f8c. Configure here. |
||
| 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 | ||
| } | ||
|
|
||
|
|
||


Uh oh!
There was an error while loading. Please reload this page.