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 pdb file for cydlib/bin on Windows in a pdb_file output group #1065

Merged
merged 2 commits into from
Dec 16, 2021

Conversation

ddeville
Copy link
Contributor

I've chose the pdb_file name for the output group since it matches what cc_binary uses
(https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java;l=707)

There is one edge case that is not covered here: if the user passes /DEBUG:NONE in the rustc_flags no pdb will be generated and I imagine the build will fail since it's now missing an output. I guess we could check the rustc_flags for it but I don't know if it's the right place to do so?

@ddeville
Copy link
Contributor Author

Looks like the test launcher adds its own OutputGroupInfo provider, let me see if those can be combined...

@UebelAndre
Copy link
Collaborator

The launcher is actually something I think we need to remove. I can ad an incompatibility flag for it

@ddeville ddeville force-pushed the windows-pdb branch 2 times, most recently from c222696 to 32518eb Compare December 14, 2021 21:45
I've chose the `pdb_file` name for the output group since it matches
what `cc_binary` uses
(https://cs.opensource.google/bazel/bazel/+/master:src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java;l=707)

There is one edge case that is not covered here: if the user passes
`/DEBUG:NONE` in the `rustc_flags` no pdb will be generated and I
imagine the build will fail since it's now missing an output. I guess we
could check the `rustc_flags` for it but I don't know if it's the right
place to do so?
@UebelAndre
Copy link
Collaborator

The launcher is actually something I think we need to remove. I can ad an incompatibility flag for it

Sorry this took so long. I've opened #1070

Copy link
Collaborator

@UebelAndre UebelAndre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for putting this together, Awesome work! 😄

Copy link
Collaborator

@UebelAndre UebelAndre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, one last change, if you don't mind 😅

@ddeville ddeville force-pushed the windows-pdb branch 2 times, most recently from 57f0e43 to f2a641d Compare December 16, 2021 19:46
Copy link
Collaborator

@UebelAndre UebelAndre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much!

@UebelAndre UebelAndre merged commit c0bdb55 into bazelbuild:main Dec 16, 2021
@ddeville ddeville deleted the windows-pdb branch December 16, 2021 20:15
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