Fixes the output path for release dsym.#35338
Fixes the output path for release dsym.#35338auto-submit[bot] merged 8 commits intoflutter:mainfrom godofredoc:fix_ios_gen
Conversation
The code path dealing with release information was not implementing the relative paths calculation correctly. Bug: flutter/flutter#81855
| subprocess.check_call([ | ||
| 'zip', '-r', | ||
| '%s/Flutter.dSYM.zip' % dst, | ||
| '%s/Flutter.dSYM/Contents' % dst |
There was a problem hiding this comment.
Is this replacing something that will be moved from the recipe? I see https://flutter.googlesource.com/recipes/+/refs/heads/main/recipes/engine/engine.py#1375 but it's zipping the Flutter.dSYM, not the Contents subdirectory.
There was a problem hiding this comment.
Nice catch, updated!
There was a problem hiding this comment.
Let me know when this is all checked in and I can validate the final output, if you want.
There was a problem hiding this comment.
Also maybe https://github.com/flutter/engine/blob/main/testing/symbols/verify_exported.dart should be updated to check this zip exists?
There was a problem hiding this comment.
I'm missing adding the verify exported check. Whether this script is run or not will depend on the configuration file. I may need to make some changes to verify_exported before I can add it to the config file. Let run it with led and share the build.
There was a problem hiding this comment.
Here is the led run https://chromium-swarm.appspot.com/task?id=5cad4aa97b75841
There was a problem hiding this comment.
Looks like an outage, I can take a look on Monday
There was a problem hiding this comment.
Here is one successful led run: https://chromium-swarm.appspot.com/task?id=5cb1a2362e463c10
There was a problem hiding this comment.
How can I download the artifacts to check if the symbols are in the right place?
There was a problem hiding this comment.
You can click the gsutil.upload links at the end of the build, they will take you to the upload location in gcs.
| 'zip', '-r', 'artifacts.zip', 'gen_snapshot_arm64', 'Flutter.xcframework' | ||
| ], | ||
| cwd=dst) | ||
| if (os.path.exists(os.path.join(dst, 'Flutter.dSYM', 'Contents'))): |
There was a problem hiding this comment.
Do the scripts already know not to generate external symbols except in release mode?
There was a problem hiding this comment.
Not sure, but for engine_v2 recipes we are calling explicit ninja targets rather than building the full configuration. This is an example: https://github.com/flutter/engine/pull/35366/files
jmagman
left a comment
jmagman
left a comment
There was a problem hiding this comment.
It's not zipping this up correctly, it's unzipping a directory named opt that contains opt/s/w/ir/cache/builder/src/out/release/
Here's from the led command:
https://storage.googleapis.com/flutter_archives_v2/flutter_infra_release/flutter/cc2b4dbc0994fafe25752f32487a7ab52ca07a8d/ios-release/Flutter.dSYM.zip
opt/s/w/ir/cache/builder/src/out/release/Flutter.dSYM/Contents/Resources/DWARF/Flutter
Here's what it's supposed to look like:
https://storage.googleapis.com/flutter_infra_release/flutter/f16e757d5d68c164d084b61d84e3b7cd14eacba9/ios-release/Flutter.dSYM.zip
Flutter.dSYM/Contents/Resources/DWARF/Flutter
|
@godofredoc Can I get another |
It took several changes to get the zip file right. Here is the link: https://chromium-swarm.appspot.com/task?id=5cbda3086da64d10 |
jmagman
left a comment
jmagman
left a comment
There was a problem hiding this comment.
zip structure looks correct now.
godofredoc
added
the
autosubmit
2 participants
The code path dealing with release information was not implementing the
relative paths calculation correctly.
Bug: flutter/flutter#81855
Pre-launch Checklist
writing and running engine tests.
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.