Skip to content

Commit e361fa6

Browse files
feat(symbols): key Android uploads by R8's map id, and upload dSYMs from an Xcode build (#769)
* feat(symbols): find the Android mapping and app version in the build `ldcli symbols upload --type android` needed a --path pointing at a directory holding mapping.txt and an --app-version matching what the app reports, so every project had to add build script code to stage the mapping somewhere and plumb its version into CI. The Android Gradle Plugin already writes both: R8's mapping at <module>/build/outputs/mapping/<variant>/mapping.txt, and the version it packaged in output-metadata.json beside the APK. Read them, and the command works from an Android project root with no flags and no build script at all. Several obfuscated variants is an error naming them rather than a guess, since only one of them is the build being shipped. An Android mapping is now also stored as mapping.txt however deep it was found. Symbolication reads it at <lane>/mapping.txt, so a mapping uploaded from a nested path was previously keyed somewhere nothing looks. Co-authored-by: Cursor <cursoragent@cursor.com> * feat(symbols): read the Android symbols id out of the packaged app A build that stamps a content-derived symbols id into assets/ld_symbols_id.txt had to also stage a mapping.txt.symbolsid sidecar next to a copy of the mapping, purely so the upload could be keyed by the same id the app reports. The packaged APK/AAB already carries that asset, and it is the app that will run: what it carries is exactly what will be reported. Reading it there needs nothing handed over by the build and leaves no way for the two to disagree, so the staging step goes away and a stamped build uploads on the Symbols Id Lane with no flags. An id that does not match the 32-hex-char shape the SDK reports is ignored rather than keyed on, since it names a lane nothing would ever ask for. Co-authored-by: Cursor <cursoragent@cursor.com> * feat(symbols): gzip what an upload sends A release R8 mapping is tens of megabytes of text and every build pushes it again: the e2e app's is 61.3 MB, which gzips to 5.0 MB in under half a second. React Native source maps and Apple symbol maps compress on the same order. Each object is marked Content-Encoding: gzip, so it stays self-describing — storage returns the header on read, an HTTP client inflates it in transit, and the backend inflates whatever still arrives compressed. Artifacts held in memory are compressed before an upload URL is asked for rather than at the point of sending, because the digest that proves an object is already stored is compared against the ETag of the stored bytes; hashing the artifact instead would never match and every source bundle would be re-sent. Files are compressed as they are read, streamed through the compressor so a large mapping is never held in memory whole. Anything that comes out of gzip no smaller is sent as it is, so a .srcbundle, which already gzips its own entries, is not wrapped a second time. Co-authored-by: Cursor <cursoragent@cursor.com> * feat(symbols): build the R8 index in the CLI instead of uploading the mapping An Android upload sent R8's mapping.txt as it was, tens of megabytes of text, and the backend parsed it into a random-access index on the first crash of every build that ever crashed. The mapping is here, on the machine that just produced it, so the index is built here too and the text is never stored: `upload` and `generate` now write mapping.v1.index to the Symbols Id and Version lanes. The mapping is streamed through r8index.EncodeFrom rather than parsed into memory, so a build machine does not have to find hundreds of megabytes to produce a few. A mapping that yields no index is an error: symbolication reads only the index, so storing anything else would leave a build looking like it has symbols while every crash arrives obfuscated. internal/symbols/r8index is a verbatim copy of the backend's package, the way dsymmap and srcbundle already are, and srcbundle.Builder now compresses on add so that a per-class bundle costs its compressed size rather than its raw one. Co-authored-by: Cursor <cursoragent@cursor.com> * test(symbols): pin the R8 index format with a golden fixture The r8index package exists twice, here and in the backend that reads what this writes, and an index is only interchangeable if the two copies agree byte for byte. Two copies cannot import each other's tests, so what they share is a fixture: a readable mapping and the exact index bytes it encodes to, identical in both repos. Either side drifting now fails here rather than in production, where the symptom would be an unreadable index or a silently wrong frame. Co-authored-by: Cursor <cursoragent@cursor.com> * test(symbols): cross-check a CLI-built index against its mapping The golden fixture proves the encoder here still produces the bytes both repos agreed on; this proves the artifact a build machine wrote answers like the mapping it came from, which is the guarantee symbolication actually rests on. Opt-in via R8_MAPPING and R8_INDEX, since it needs a real release build. Co-authored-by: Cursor <cursoragent@cursor.com> * test(symbols): name the real mapping to check against with a flag Keeps the r8index copy in step with the backend's, where reading these paths from R8_MAPPING and R8_INDEX fails a CI check that bans os.Getenv anywhere in the backend. A test flag is what the package already reaches for anyway (-update, for the golden fixture). end-of-file-fixer runs here too, through pre-commit, and it appended a newline to the backend's copy of the golden index — a byte that moves the footer the reader locates the index by, and breaks the byte-for-byte agreement the fixture exists to prove. It cannot tell a binary file from a text one, so *.index is excluded. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(symbols): accept --source-path on `symbols generate` Generating an Android source bundle reads sourcePathFlag, which generate never registered: --source-path was rejected outright, so sources could only be scanned from the working directory. The read answered anyway, because both commands bind the same viper keys and upload's flag was left to supply the default — a value the caller of generate could neither see nor change. Also stop describing --include-sources as Apple-only, since an Android mapping carries sources now too. Co-authored-by: Cursor <cursoragent@cursor.com> * feat(symbols): key an Android upload by the id R8 recorded for the mapping An Android build only reached the Symbols Id Lane if something had stamped an id into the app for it, which meant a Gradle task in every project that wanted one. R8 has been recording an id for its own mapping all along — "# pg_map_id:" in the header — and from AGP 8.12 it stamps that same id into each class, so the shipped app already reports it on every frame of every crash. Read it from the header when no id was given or found in the packaged app, and key the upload by it. A project that adds nothing to its build now uploads to the lane its crashes arrive on; an id the app stamped still wins, since that is a build saying what it will report, and the only answer for one too old for R8 to stamp anything itself. Co-authored-by: Cursor <cursoragent@cursor.com> * feat(symbols): upload the dSYMs an Xcode build just produced A dSYM is best uploaded by the build that made it, which means a Run Script phase — and every project writing one wrote the same two pieces of shell first: a --path pointing at DWARF_DSYM_FOLDER_PATH, and a guard skipping the configurations that make no dSYM, without which a Debug build fails on "no .dSYM bundles found". Both are debugged inside a build phase, where the way you find out is a failed build. Read the folder from the build environment when no --path was given, and treat finding no dSYM there as nothing to do rather than as an error. What a phase has to say is now the project it uploads to. An explicit --path still wins, so uploading a dSYM from an archive or from CI is unchanged. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(symbols): one index per mapping when a class name repeats Mirrors the observability copy of r8index, which has to stay byte-identical apart from the srcbundle import path. srcbundle.Builder.Add ignores a key it already holds, so streaming keeps the first block for a repeated obfuscated name, while Parse's map assignment kept the last. One mapping then encoded to two different indexes depending on which encoder read it, which is not something an artifact addressed by its mapping's id can afford. Parse keeps the first too: streaming has no other option, since a class is gzipped and handed off before a duplicate arrives. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 404c4cc commit e361fa6

10 files changed

Lines changed: 338 additions & 13 deletions

File tree

cmd/symbols/android_mapid.go

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
package symbols
2+
3+
import (
4+
"bufio"
5+
"os"
6+
"regexp"
7+
"strings"
8+
)
9+
10+
// The id R8 gives the mapping it produced.
11+
//
12+
// R8 writes "# pg_map_id: <hash of the mapping>" into every mapping's header, and
13+
// from AGP 8.12 it stamps that same id into each class's source file attribute — so
14+
// a shipped app reports "r8-map-id-<hash>" where a file name goes, on every frame of
15+
// every crash. Keying the upload by it puts the index exactly where symbolication
16+
// will look for a build that was asked to do nothing at all.
17+
//
18+
// It is read out of the header rather than recomputed here because the header is
19+
// R8's own statement of what it stamped. A hash computed on this side would have to
20+
// agree with R8's forever, and would be silently wrong the first time it didn't.
21+
22+
// androidMapIDComment is the mapping header line carrying the id.
23+
const androidMapIDComment = "# pg_map_id:"
24+
25+
// androidMapIDPattern is the shape of an id that may be keyed by: a hash, which is
26+
// the mapping's full SHA-256 from AGP 8.12 and a 7-character prefix of it before.
27+
var androidMapIDPattern = regexp.MustCompile(`^[0-9a-f]{7,64}$`)
28+
29+
// androidMapID returns the id R8 recorded for a mapping, or "" when it recorded none
30+
// — a mapping written by ProGuard rather than R8, or one old enough to predate the
31+
// header. Best effort, like every other way of learning what a build shipped: an
32+
// upload with no id falls back to the Version Lane rather than failing over it.
33+
func androidMapID(mappingPath string) string {
34+
file, err := os.Open(mappingPath)
35+
if err != nil {
36+
return ""
37+
}
38+
defer file.Close()
39+
40+
scanner := bufio.NewScanner(file)
41+
for scanner.Scan() {
42+
line := scanner.Text()
43+
// The header is the comments before the first class, and a release mapping
44+
// is tens of megabytes of what follows it.
45+
if !strings.HasPrefix(line, "#") {
46+
return ""
47+
}
48+
rest, ok := strings.CutPrefix(line, androidMapIDComment)
49+
if !ok {
50+
continue
51+
}
52+
if id := strings.TrimSpace(rest); androidMapIDPattern.MatchString(id) {
53+
return id
54+
}
55+
return ""
56+
}
57+
return ""
58+
}

cmd/symbols/android_mapid_test.go

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
package symbols
2+
3+
import (
4+
"path/filepath"
5+
"testing"
6+
7+
"github.com/stretchr/testify/assert"
8+
"github.com/stretchr/testify/require"
9+
)
10+
11+
// The header R8 8.13 writes, trimmed to what is read from it.
12+
const testMapIDHeader = `# compiler: R8
13+
# compiler_version: 8.13.19
14+
# min_api: 23
15+
# common_typos_disable
16+
# {"id":"com.android.tools.r8.mapping","version":"2.2"}
17+
# pg_map_id: 92d0222f1a7a3b92fca00ddc75fbcf893c89be03e02286414d51abcfd9b02063
18+
# pg_map_hash: SHA-256 92d0222f1a7a3b92fca00ddc75fbcf893c89be03e02286414d51abcfd9b02063
19+
`
20+
21+
const testMapID = "92d0222f1a7a3b92fca00ddc75fbcf893c89be03e02286414d51abcfd9b02063"
22+
23+
func TestAndroidMapIDReadsTheHeader(t *testing.T) {
24+
path := writeMappingFile(t, testMapIDHeader+testAndroidMapping)
25+
assert.Equal(t, testMapID, androidMapID(path))
26+
}
27+
28+
// R8 shortened the id before AGP 8.12, and a build being retraced today may well
29+
// have been produced by one of those.
30+
func TestAndroidMapIDReadsAShortID(t *testing.T) {
31+
path := writeMappingFile(t, "# compiler: R8\n# pg_map_id: 92d0222\n"+testAndroidMapping)
32+
assert.Equal(t, "92d0222", androidMapID(path))
33+
}
34+
35+
// Nothing here may fail an upload: an id that cannot be read leaves the mapping on
36+
// the Version Lane, which is where it was before R8 recorded one.
37+
func TestAndroidMapIDWithoutOne(t *testing.T) {
38+
cases := map[string]string{
39+
"no header at all": testAndroidMapping,
40+
"a ProGuard mapping": "com.example.app.CheckoutDemo -> a.b.c:\n",
41+
"an id that is not a hash": "# compiler: R8\n# pg_map_id: release-7\n" + testAndroidMapping,
42+
"an empty id": "# compiler: R8\n# pg_map_id:\n" + testAndroidMapping,
43+
}
44+
for name, mapping := range cases {
45+
assert.Emptyf(t, androidMapID(writeMappingFile(t, mapping)), "androidMapID of %s", name)
46+
}
47+
48+
assert.Empty(t, androidMapID(filepath.Join(t.TempDir(), "nothing-here.txt")))
49+
}
50+
51+
// The header ends at the first class, and what follows is tens of megabytes of a
52+
// build's own strings — including, in an app that has one, a class whose name would
53+
// read as a header line to anything still looking.
54+
func TestAndroidMapIDStopsAtTheFirstClass(t *testing.T) {
55+
path := writeMappingFile(t, testAndroidMapping+"# pg_map_id: "+testMapID+"\n")
56+
assert.Empty(t, androidMapID(path))
57+
}
58+
59+
// The point of the id: a build that stamps nothing and is told nothing still uploads
60+
// to the lane its crashes will arrive on, because R8 recorded which mapping this is
61+
// and the shipped app reports the same thing on every frame.
62+
func TestBuildAndroidObjectsKeysByTheMapID(t *testing.T) {
63+
path := writeMappingFile(t, testMapIDHeader+testAndroidMapping)
64+
65+
objects, err := buildAndroidObjects(path, "", "", false, "")
66+
require.NoError(t, err)
67+
require.Len(t, objects, 1)
68+
assert.Equal(t, "_sym/android/id/"+testMapID+"/mapping.v1.index", objects[0].Key())
69+
assert.True(t, objects[0].keyProvesContent, "the key is derived from the mapping it stores")
70+
}
71+
72+
// An id the caller gave, or one read out of the packaged app, is a build saying what
73+
// it will report — which is the more specific answer, and the only one for a build
74+
// old enough that R8 stamps nothing into the app itself.
75+
func TestBuildAndroidObjectsPrefersAReportedID(t *testing.T) {
76+
path := writeMappingFile(t, testMapIDHeader+testAndroidMapping)
77+
78+
objects, err := buildAndroidObjects(path, "", "deadbeef", false, "")
79+
require.NoError(t, err)
80+
require.Len(t, objects, 1)
81+
assert.Equal(t, "_sym/android/id/deadbeef/mapping.v1.index", objects[0].Key())
82+
}

cmd/symbols/android_upload.go

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -126,15 +126,25 @@ func buildAndroidObjects(path, appVersion, symbolsID string, includeSources bool
126126
return nil, err
127127
}
128128

129-
lanes := androidLanes(build)
130-
if len(lanes) == 0 {
131-
return nil, fmt.Errorf("this build reports no symbols id and no app version, so there is no key a crash could be symbolicated under. Apply the LaunchDarkly Gradle plugin so the shipped app records its symbols id, or re-run with --%s <version>", appVersionFlag)
132-
}
133-
134129
mapping, err := findAndroidMapping(build.Path)
135130
if err != nil {
136131
return nil, err
137132
}
133+
if build.SymbolsID == "" {
134+
// R8's own id for this mapping, which the shipped app reports on every frame
135+
// (see android_mapid.go). Below an id the app stamped, which is a build
136+
// saying what it will report and so the more specific answer of the two.
137+
if id := androidMapID(mapping); id != "" {
138+
build.SymbolsID = id
139+
fmt.Printf("Using symbols id %s, as recorded by R8 in the mapping\n", id)
140+
}
141+
}
142+
143+
lanes := androidLanes(build)
144+
if len(lanes) == 0 {
145+
return nil, fmt.Errorf("this mapping records no id of its own and the build reports no app version, so there is no key a crash could be symbolicated under. Re-run with --%s <version>", appVersionFlag)
146+
}
147+
138148
index, err := buildAndroidIndex(mapping)
139149
if err != nil {
140150
return nil, err

cmd/symbols/apple_upload.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,21 @@ func (m appleSymbolMap) label() string {
5454
// nothing. Source bundles borrow that UUID rather than being keyed by their own
5555
// contents — sources unreadable on the machine that uploaded first must still be able
5656
// to overwrite — so those are skipped only when their digest matches what is stored.
57-
func uploadAppleDSYMs(apiKey, projectID, path, backendURL string, includeSources, skipExisting bool) error {
58-
images, err := findDSYMImages(path)
57+
func uploadAppleDSYMs(apiKey, projectID string, upload appleUpload, backendURL string, includeSources, skipExisting bool) error {
58+
images, err := findDSYMImages(upload.Path)
5959
if err != nil {
6060
return fmt.Errorf("failed to find dSYM files: %w", err)
6161
}
6262
if len(images) == 0 {
63-
return fmt.Errorf("no .dSYM bundles found in %s, is this the correct path?", path)
63+
if upload.FromXcode {
64+
// Running from a build that produced no dSYM is ordinary — a Debug
65+
// build's debug information stays in the binary — and a build phase
66+
// that fails the build over it would be a phase every project has to
67+
// guard. There is nothing to upload, which is not the same as an error.
68+
fmt.Printf("This build produced no dSYM, so there is nothing to upload. Set Debug Information Format to \"DWARF with dSYM File\" for the configurations you ship.\n")
69+
return nil
70+
}
71+
return fmt.Errorf("no .dSYM bundles found in %s, is this the correct path?", upload.Path)
6472
}
6573

6674
maps, err := buildAppleMaps(images, includeSources)

cmd/symbols/apple_xcode.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
package symbols
2+
3+
import (
4+
"fmt"
5+
"os"
6+
)
7+
8+
// Uploading from inside an Xcode build.
9+
//
10+
// The reliable moment to upload a dSYM is the build that produced it, which means
11+
// a Run Script phase. Xcode runs one with the whole build's settings in its
12+
// environment, so the phase can be told nothing and still know everything:
13+
// DWARF_DSYM_FOLDER_PATH is where this build put its dSYMs.
14+
//
15+
// Reading it here is what keeps the phase to a single line. Otherwise every
16+
// project writes the same two pieces of shell — a path that has to stay in step
17+
// with the project's configuration, and a guard against the configurations that
18+
// produce no dSYM — and gets to debug them inside a build phase, where the way you
19+
// find out is a failed build.
20+
21+
// xcodeDSYMFolderEnv is the build setting naming the folder Xcode wrote this
22+
// build's dSYMs to. It is set for every configuration, including the ones whose
23+
// debug information format produces no dSYM at all.
24+
const xcodeDSYMFolderEnv = "DWARF_DSYM_FOLDER_PATH"
25+
26+
// appleUpload is where an Apple upload reads its dSYMs from.
27+
type appleUpload struct {
28+
Path string
29+
// FromXcode records that the path came from the surrounding build rather than
30+
// from the caller, which is what makes finding no dSYM there ordinary: it is
31+
// the answer for a Debug build, and not something to fail over.
32+
FromXcode bool
33+
}
34+
35+
// resolveAppleUpload decides where to read dSYMs from. An explicit --path always
36+
// wins — a phase that wants to upload something other than what it just built can
37+
// still say so — and the build environment answers when there was no path to go on.
38+
func resolveAppleUpload(path string) appleUpload {
39+
if path != defaultPath {
40+
return appleUpload{Path: path}
41+
}
42+
43+
folder := os.Getenv(xcodeDSYMFolderEnv)
44+
if folder == "" {
45+
return appleUpload{Path: path}
46+
}
47+
48+
fmt.Printf("Using the dSYMs this Xcode build produced, from %s\n", folder)
49+
return appleUpload{Path: folder, FromXcode: true}
50+
}

cmd/symbols/apple_xcode_test.go

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
package symbols
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/assert"
7+
"github.com/stretchr/testify/require"
8+
)
9+
10+
// The point of reading the build environment: a phase that says nothing at all
11+
// still uploads what the build it is part of just produced.
12+
func TestResolveAppleUploadFromXcode(t *testing.T) {
13+
t.Setenv(xcodeDSYMFolderEnv, "/dd/Build/Products/Release-iphoneos")
14+
15+
upload := resolveAppleUpload(defaultPath)
16+
assert.Equal(t, "/dd/Build/Products/Release-iphoneos", upload.Path)
17+
assert.True(t, upload.FromXcode)
18+
}
19+
20+
// A phase that names a path means it: uploading a dSYM from somewhere other than
21+
// the build in progress has to stay possible.
22+
func TestResolveAppleUploadPrefersAnExplicitPath(t *testing.T) {
23+
t.Setenv(xcodeDSYMFolderEnv, "/dd/Build/Products/Release-iphoneos")
24+
25+
upload := resolveAppleUpload("./archives/MyApp.xcarchive/dSYMs")
26+
assert.Equal(t, "./archives/MyApp.xcarchive/dSYMs", upload.Path)
27+
assert.False(t, upload.FromXcode)
28+
}
29+
30+
func TestResolveAppleUploadOutsideXcode(t *testing.T) {
31+
t.Setenv(xcodeDSYMFolderEnv, "")
32+
33+
upload := resolveAppleUpload(defaultPath)
34+
assert.Equal(t, defaultPath, upload.Path)
35+
assert.False(t, upload.FromXcode)
36+
}
37+
38+
// Xcode sets the folder for every configuration, including the ones whose debug
39+
// information stays in the binary. Finding no dSYM there is the answer for a Debug
40+
// build, and a build phase that failed the build over it would be one every project
41+
// has to write a guard around.
42+
func TestUploadAppleDSYMsFromXcodeWithoutADSYM(t *testing.T) {
43+
err := uploadAppleDSYMs("", "", appleUpload{Path: t.TempDir(), FromXcode: true}, "", false, false)
44+
assert.NoError(t, err)
45+
}
46+
47+
// Asked for a path with no dSYM under it, though, there is nothing else this could
48+
// have meant, so it is still reported.
49+
func TestUploadAppleDSYMsWithoutADSYM(t *testing.T) {
50+
dir := t.TempDir()
51+
52+
err := uploadAppleDSYMs("", "", appleUpload{Path: dir}, "", false, false)
53+
require.Error(t, err)
54+
assert.Contains(t, err.Error(), dir)
55+
}

cmd/symbols/upload.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -224,8 +224,11 @@ func runE(client resources.Client) func(cmd *cobra.Command, args []string) error
224224
// Apple dSYMs take a dedicated path: they are compiled to per-arch .dsymmap
225225
// symbol maps keyed by build UUID, ignoring the version/symbols-id lanes.
226226
if symbolType == typeAppleDSYM {
227-
fmt.Printf("Starting to upload %s symbols from %s\n", symbolType, path)
228-
return uploadAppleDSYMs(viper.GetString(cliflags.AccessTokenFlag), projectResult.ID, path, backendUrl, viper.GetBool(includeSourcesFlag), skipExisting)
227+
// A dSYM is best uploaded by the build that produced it, so where to
228+
// read one from can come from the build itself. See apple_xcode.go.
229+
upload := resolveAppleUpload(path)
230+
fmt.Printf("Starting to upload %s symbols from %s\n", symbolType, upload.Path)
231+
return uploadAppleDSYMs(viper.GetString(cliflags.AccessTokenFlag), projectResult.ID, upload, backendUrl, viper.GetBool(includeSourcesFlag), skipExisting)
229232
}
230233

231234
// Flutter/Dart symbols take a dedicated path too: each app.<platform>.symbols
@@ -700,10 +703,10 @@ func initFlags(cmd *cobra.Command) {
700703
cmd.Flags().String(appVersionFlag, "", fmt.Sprintf("The current version of your deploy. With --type %s this is read from the packaged build when omitted", typeAndroid))
701704
_ = viper.BindPFlag(appVersionFlag, cmd.Flags().Lookup(appVersionFlag))
702705

703-
cmd.Flags().String(symbolsIdFlag, "", fmt.Sprintf("The symbols id (launchdarkly.symbols_id.htlhash) to key uploads by (Symbols Id Lane). If omitted, a *.symbolsid sidecar next to the bundle is used when present, and with --type %s the id the packaged app reports", typeAndroid))
706+
cmd.Flags().String(symbolsIdFlag, "", fmt.Sprintf("The symbols id (launchdarkly.symbols_id.htlhash) to key uploads by (Symbols Id Lane). If omitted, a *.symbolsid sidecar next to the bundle is used when present, and with --type %s the id the packaged app reports, or failing that the one R8 recorded in the mapping", typeAndroid))
704707
_ = viper.BindPFlag(symbolsIdFlag, cmd.Flags().Lookup(symbolsIdFlag))
705708

706-
cmd.Flags().String(pathFlag, defaultPath, fmt.Sprintf("Sets the directory of where the symbol files are. With --type %s, run from your project root and the R8 mapping is found for you", typeAndroid))
709+
cmd.Flags().String(pathFlag, defaultPath, fmt.Sprintf("Sets the directory of where the symbol files are. With --type %s, run from your project root and the R8 mapping is found for you; with --type %s, an Xcode build phase uploads what it just built", typeAndroid, typeAppleDSYM))
707710
_ = viper.BindPFlag(pathFlag, cmd.Flags().Lookup(pathFlag))
708711

709712
cmd.Flags().String(basePathFlag, "", "An optional base path for the uploaded symbol files")

internal/symbols/r8index/encode.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,9 @@ func EncodeFrom(r io.Reader) ([]byte, error) {
5555
var b srcbundle.Builder
5656
classes := 0
5757
sc := newScanner(func(obf string, c *class) {
58+
// Add ignores a key it already holds, so a repeated obfuscated name keeps its
59+
// first block. Parse keeps the first too (see there), because both encoders
60+
// have to produce one index for one mapping.
5861
b.Add(classPrefix+obf, encodeClass(c))
5962
classes++
6063
})

internal/symbols/r8index/parse.go

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,20 @@ func (m *Mapping) Classes() int {
7272
// check.
7373
func Parse(data []byte) *Mapping {
7474
m := &Mapping{classes: map[string]*class{}}
75-
sc := newScanner(func(obf string, c *class) { m.classes[obf] = c })
75+
// The first block wins a repeated obfuscated name, which is the one thing
76+
// EncodeFrom can do: srcbundle.Builder.Add ignores a key it already holds, and a
77+
// class it has already compressed into the bundle cannot be revisited. Plain
78+
// assignment here would keep the last instead, so one mapping would encode to two
79+
// different indexes depending on which encoder read it — and an index is
80+
// addressed by the id of the mapping it was built from.
81+
//
82+
// A name repeats when mapping files are concatenated: a build with feature splits,
83+
// or one that appends a module's mapping to another's.
84+
sc := newScanner(func(obf string, c *class) {
85+
if _, seen := m.classes[obf]; !seen {
86+
m.classes[obf] = c
87+
}
88+
})
7689
sc.scan(data)
7790
m.sourceFiles = sc.sourceFiles
7891
return m

0 commit comments

Comments
 (0)