bazel icon indicating copy to clipboard operation
bazel copied to clipboard

Add DUMPBIN make variable to MSVC C/C++ toolchain

Open alexsharoff opened this issue 1 year ago • 4 comments

DUMPBIN is a WIndows/MSVC tool that provides similar function to NM in the Linux world. NM already has a make variable, so I think it's fair to add one for DUMPBIN.

Tests and docs are updated as well.

RELNOTES[NEW]: New $(DUMPBIN) make variable is now available for Visual Studio toolchains.

alexsharoff avatar Feb 07 '24 06:02 alexsharoff

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

google-cla[bot] avatar Feb 07 '24 06:02 google-cla[bot]

Any pointers on how to get the checks to succeed? I've got two failing tests, //src/test/shell/integration:discard_analysis_cache_test and //src/test/tools/bzlmod:verify_default_lock_file. How did my changes break these tests? They seem unrelated.

alexsharoff avatar Feb 09 '24 07:02 alexsharoff

bazel run //src/test/tools/bzlmod:update_default_lock_file should fix this. It's necessary because it contains a hash of certain files in @bazel_tools.

fmeum avatar Feb 09 '24 12:02 fmeum

If I fix the lock file conflict without a verdict from Bazel team regarding my changes, I'll just get the same conflict again in a day or two. I'll be waiting for now.

alexsharoff avatar Feb 14 '24 15:02 alexsharoff

Is there any chance that this PR will be reviewed in 2024?

alexander-shiryaev avatar Feb 27 '24 19:02 alexander-shiryaev

@comius @meteorcloudy

fmeum avatar Feb 27 '24 20:02 fmeum

@alexsharoff Can you please run bazel run //src/test/tools/bzlmod:update_default_lock_file again? This PR looks good to me!

meteorcloudy avatar Feb 28 '24 12:02 meteorcloudy

@alexsharoff Can you please run bazel run //src/test/tools/bzlmod:update_default_lock_file again? This PR looks good to me!

Done!

iancha1992 avatar Feb 29 '24 01:02 iancha1992

Thank you everyone!

alexsharoff avatar Feb 29 '24 13:02 alexsharoff

@iancha1992 @meteorcloudy I think this change was not merged correctly. The PR added DUMPBIN to the documentation, but the actual landed commit https://github.com/bazelbuild/bazel/commit/42611f9fd8470e6ee42bf596d20201ded07cc427 removed variables next to DUMPBIN in the docs.

Could you please double-check?

sluongng avatar Jun 21 '24 13:06 sluongng

Thanks for catching this, there was some problem during code transformation, I'm sending a fix now!

meteorcloudy avatar Jun 21 '24 13:06 meteorcloudy