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

Update /MD flag description to reflect modern behavior #5125

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

EterDelta
Copy link

The current documentation for the /MD flag states that applications compiled with this option are statically linked to MSVCRT.lib, which resolves external references to MSVCR.DLL.
However, this is outdated and misleading. In modern VS versions, MSVCRT.lib will prefer the UCRT and redirect to the UCRTBASE.DLL and VCRUNTIME.DLL.
You can check this by compiling a simple binary and dumping its dependencies.

This PR updates the /MD description to clarify where the "actual working code" is now.

Copy link
Contributor

@EterDelta : Thanks for your contribution! The author(s) have been notified to review your proposed change.

Copy link
Contributor

Learn Build status updates of commit 9c20ce7:

✅ Validation status: passed

File Status Preview URL Details
docs/build/reference/md-mt-ld-use-run-time-library.md ✅Succeeded

For more details, please refer to the build report.

For any questions, please:

@Court72
Copy link
Contributor

Court72 commented Nov 18, 2024

@TylerMSFT

Can you review the proposed changes?

Important: When the changes are ready for publication, adding a #sign-off comment is the best way to signal that the PR is ready for the review team to merge.

#label:"aq-pr-triaged"
@MicrosoftDocs/public-repo-pr-review-team

@prmerger-automator prmerger-automator bot added the aq-pr-triaged Tracking label for the PR review team label Nov 18, 2024
@TylerMSFT
Copy link
Collaborator

@elterdelta, I'm having misgivings about this page in general. Your correction is right, for a certain time period. The names changed in VS 2015. Because people are still also using versions before that, we'd need to also keep the old names and indicate that the new ones come into play in VS 2015 and later.

What I have misgivings about is that we talk about the actual names at all. During VS 2015, the effort with api sets was to create an abstraction between what you linked against and where the code actually lived. Running dumpbin.exe on a simple console exe would likely show you linking against things like api-ms-win-crt-string-l1-1-0.dll. That will ultimately resolve to ucrtbase.dll, which Windows ships starting in Windows 10, I think.
I wonder if this page should get out of the business of listing the library names and instead say 'the multithreaded version of the runtime library'. Then, the page could link to the article about the C++ redist, which explains how to install all the runtime dependencies.
You've fixed /MD, but I suspect there are similar issues with a couple of the other flags in this article.
I'm not asking you to do the fixup. But I'm wondering what you'd think about closing this PR and I'll add a work item for myself to do this cleanup.

Copy link
Contributor

Learn Build status updates of commit c438ea0:

✅ Validation status: passed

File Status Preview URL Details
docs/build/reference/md-mt-ld-use-run-time-library.md ✅Succeeded

For more details, please refer to the build report.

For any questions, please:

@EterDelta
Copy link
Author

Thanks for the context Tyler.

I wonder if this page should get out of the business of listing the library names and instead say 'the multithreaded version of the runtime library'. Then, the page could link to the article about the C++ redist, which explains how to install all the runtime dependencies.

I don't think linking the C++ redist article is necessary.
The page already links to the CRT and STL libraries article, which already explains you about the UCRT, what was used before, and redistributions:

For more information about C run-time libraries and which libraries are used when you compile with [/clr (Common Language Runtime Compilation)](clr-common-language-runtime-compilation.md), see [CRT Library Features](../../c-runtime-library/crt-library-features.md).

You've fixed /MD, but I suspect there are similar issues with a couple of the other flags in this article.

The other flags descriptions are concise, they don't mention any dependencies. This one being overly descriptive is the problem.

I think the best approach here would be to simply remove this unnecessary listing from the /MD description as you said. That way it just looks like the other flags.
We can always add more clarification later if needed.

@TylerMSFT Let me know what you think. I've ammended the commit for simplicity.

@TylerMSFT
Copy link
Collaborator

Thank you, @EterDelta . Huge storm event where I live. Power was out a couple days. Now heading into the Thanksgiving holiday so I'll be out for a week. I'll update this when I get back. I hope your Thanksgiving holiday goes well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants