Skip to content
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

Set target.swift-* lldb settings #836

Merged
merged 1 commit into from
Aug 2, 2022

Conversation

brentleyjones
Copy link
Contributor

@brentleyjones brentleyjones commented Jul 28, 2022

Part of #617.

This allows lldb to work correctly if using the -noserialize-debugging-options swiftc flag.

@brentleyjones brentleyjones marked this pull request as draft July 28, 2022 21:17
@brentleyjones brentleyjones force-pushed the bj/set-target.swift-lldb-settings branch 2 times, most recently from df80de1 to 3a7390b Compare July 28, 2022 21:38
@brentleyjones brentleyjones marked this pull request as ready for review July 28, 2022 21:39
@brentleyjones
Copy link
Contributor Author

brentleyjones commented Jul 29, 2022

Almost there. These two keys don't match. Trying to figure out when we need to add the minimum os to the triple:

lldb: "arm64-apple-macosx11.0.0 tests.xctest/Contents/MacOS/tests"
generated: "arm64-apple-macosx tests.xctest/Contents/MacOS/tests"

@brentleyjones
Copy link
Contributor Author

I swear I was seeing the triples without the os version before. Now I only see the one with os version. Maybe I can add a fallback key lookup just in case...

@brentleyjones brentleyjones force-pushed the bj/set-target.swift-lldb-settings branch from 3a7390b to c2cb45b Compare July 29, 2022 13:35
@brentleyjones
Copy link
Contributor Author

Regardless, latest issue is that these two don't match:

lldb: "arm64-apple-macosx11.0.0 tests.xctest/Contents/MacOS/tests"
generated: "arm64-apple-macosx12.0.0 tests.xctest/Contents/MacOS/tests"

I have --macos_minimum_os=12.0 set, so I'm not sure what's up with that yet.

@brentleyjones brentleyjones marked this pull request as draft July 29, 2022 13:40
@brentleyjones
Copy link
Contributor Author

brentleyjones commented Jul 29, 2022

And now it works. Hmmmmm 🤔. I'm thinking it was some sort of caching issue (multiple Xcode projects open, both of them stopped in lldb...), so we just need the one backup now, though maybe I can convert the lldb one to the generic one when doing that. The downside is the small chance of collisions if there are multiple versions of executable paths that only differ on os version, but the backup is better than nothing.

@brentleyjones brentleyjones force-pushed the bj/set-target.swift-lldb-settings branch from c2cb45b to f595e70 Compare July 29, 2022 14:31
@brentleyjones brentleyjones marked this pull request as ready for review July 29, 2022 14:31
@brentleyjones
Copy link
Contributor Author

Fallback added, I think this is good to go.

@jpsim It would be nice to make sure that this doesn't regress debugging for EM. It doesn't set the flag that removes debug information (--features=swift.cacheable_swiftmodules) but EM already sets that, so this should only improve debugging of Swift code for you.

@cgrindel can you test this on your internal project as well?

@brentleyjones
Copy link
Contributor Author

(And I found the 11.0 vs 12.0, and it's related to generator's tests. Digging in.)

@brentleyjones brentleyjones force-pushed the bj/set-target.swift-lldb-settings branch from f595e70 to 6d61bc9 Compare July 29, 2022 14:43
@brentleyjones
Copy link
Contributor Author

And now that is fixed! Was looking at yet another wrong place for the triple. So many places to get it in lldb...

@brentleyjones brentleyjones force-pushed the bj/set-target.swift-lldb-settings branch 2 times, most recently from 93a93fa to 08626b7 Compare July 29, 2022 20:49
@jpsim
Copy link
Contributor

jpsim commented Jul 30, 2022

Lightweight debugging testing with EM seems to still work. I didn't review the changes here in depth though, there's a lot that was touched.

@brentleyjones brentleyjones force-pushed the bj/set-target.swift-lldb-settings branch 2 times, most recently from 112d9ce to 638aa0d Compare August 1, 2022 12:32
This allows lldb to work correctly if using the `-serialize-debugging-options` `swiftc` flag.
@brentleyjones brentleyjones force-pushed the bj/set-target.swift-lldb-settings branch from 638aa0d to 43df10f Compare August 2, 2022 15:48
@brentleyjones
Copy link
Contributor Author

I got a couple offline reports of this working, so merging.

@brentleyjones brentleyjones merged commit a78fc88 into main Aug 2, 2022
@brentleyjones brentleyjones deleted the bj/set-target.swift-lldb-settings branch August 2, 2022 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants