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

Provide MSVC compiler PDB support #78

Closed
wants to merge 1 commit into from

Conversation

tomaszstrejczek
Copy link

Bazel already contains support for link-time .PDB files.
The change uses fission to provide compile-time .PDB files support.

@wrowe
Copy link
Contributor

wrowe commented Nov 12, 2020

We have been force-setting /Z7 on opt builds, in order to feed symbolic debugging information through the .obj files. This proposal looks very interesting! Our concern would revolve around being able to toggle it for opt builds.

Or second concern is carrying a massive amount of debugging weight on dbg builds in a typical //test/... scenario of a thousand tests. Those don't necessarily need symbolic debug info, so being able to toggle this off for specific groups of objects would be helpful. But all these comments are enhancement requests, looking forward to testing it out in practice at envoy.

@sunjayBhatia
Copy link
Contributor

@oquenchil do you have any thoughts on this?

@wrowe
Copy link
Contributor

wrowe commented Dec 14, 2020

I have one new thought. What if we introduced logic at cl command construction time to rewind over the earlier args, if /Z0 (zero) is presented, and delete all /Z[7Ii] arguments encounter (along with the /Z0 and any which incidently trigger those flags)? This might even work for constructing the rules_foreign_cc copts.

It would just be a matter of not taking the absence of a resulting .pdb as an error, then this patch would seamlessly work for objects generated Z7, Zi, or Z0

@oquenchil
Copy link
Collaborator

Closing this stale PR. In general we have been considering recently taking the current state of these duplicated files from bazel_tools, taking bazel_tools as the current correct state but making rules_cc the only source of truth.

@oquenchil oquenchil closed this Mar 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants