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

switch to dynamic linked VCRT #657

Merged
merged 8 commits into from
Aug 27, 2024
Merged

switch to dynamic linked VCRT #657

merged 8 commits into from
Aug 27, 2024

Conversation

mtfriesen
Copy link
Contributor

@mtfriesen mtfriesen commented Aug 22, 2024

Technically a breaking change. We need to use dynamic linking to VCRT in our internal, official builds for the debug flavor, so we should use it everywhere.

@mtfriesen mtfriesen added the build Related to building/compiling the code label Aug 22, 2024
@mtfriesen mtfriesen requested a review from a team as a code owner August 22, 2024 15:46
@nibanks
Copy link
Member

nibanks commented Aug 22, 2024

I thought we were only having a problem internally for DEBUG builds? Could we use dynamic CRT only for debug?

@mtfriesen
Copy link
Contributor Author

I thought we were only having a problem internally for DEBUG builds? Could we use dynamic CRT only for debug?

I believe we're always linking with DLLs internally, which is the fundamental problem - we've got different build configurations internally and externally. This aligns all our builds to be the same.

@mtfriesen mtfriesen enabled auto-merge (squash) August 22, 2024 17:13
@nibanks
Copy link
Member

nibanks commented Aug 23, 2024

I thought we were only having a problem internally for DEBUG builds? Could we use dynamic CRT only for debug?

I believe we're always linking with DLLs internally, which is the fundamental problem - we've got different build configurations internally and externally. This aligns all our builds to be the same.

Internally should be matching Windows internal builds, which is to partially statically link the CRT (which we also do in Windows).

@mtfriesen
Copy link
Contributor Author

Internally should be matching Windows internal builds, which is to partially statically link the CRT (which we also do in Windows).

I'm just describing how OneBranch is actually building our binaries, e.g. xdpapi.dll imports:

    ucrtbased.dll
             180006110 Import Address Table
             180007250 Import Name Table
                     0 time date stamp
                     0 Index of first forwarder reference

                          1A _CrtSetReportFile
                          1E _CrtSetReportMode
                         174 _initterm
                         175 _initterm_e
                         11D _free_dbg
                          A3 _calloc_dbg
                         330 _o__cexit
                         33D _o__configure_narrow_argv
                         35F _o__execute_onexit_table
                         3D9 _o__initialize_narrow_environment
                         3DA _o__initialize_onexit_table
                         4F6 _o__seh_filter_dll
                         5DE _o_abort
                         6D2 _o_terminate

The point of this PR is mainly to replicate the same missing debug CRT issue we are hitting on internal builds, and to provide a solution. The work to take a dependency on the framework-for-a-framework is #605 which is lower priority.

nibanks
nibanks previously approved these changes Aug 27, 2024
nibanks
nibanks previously approved these changes Aug 27, 2024
@mtfriesen mtfriesen merged commit 3ceb8db into main Aug 27, 2024
31 checks passed
@mtfriesen mtfriesen deleted the mtfriesen/debugcrt branch August 27, 2024 20:01
@ami-GS ami-GS mentioned this pull request Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Related to building/compiling the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants