Skip to content

Disable force_install warning produced by CMake for libraries native build.#32853

Merged
safern merged 8 commits into
masterfrom
Vs2019PreviewOfficialBuild
Feb 27, 2020
Merged

Disable force_install warning produced by CMake for libraries native build.#32853
safern merged 8 commits into
masterfrom
Vs2019PreviewOfficialBuild

Conversation

@safern
Copy link
Copy Markdown
Member

@safern safern commented Feb 26, 2020

This lets us enable warn as error in libraries build.

Fixes: #32876

cc: @dotnet/runtime-infrastructure

This lets us enable warn as error in libraries build because this pools contain a newer CMake.
@jkotas
Copy link
Copy Markdown
Member

jkotas commented Feb 26, 2020

Is it really a good idea to use different compilers for CI vs. for official builds? It just means that there are more ways to break official builds that won't be caught by the CI.

@safern
Copy link
Copy Markdown
Member Author

safern commented Feb 26, 2020

This is actually getting CI and official builds to use the same compiler. I merged my CI update earlier today. #32626

@jkotas
Copy link
Copy Markdown
Member

jkotas commented Feb 26, 2020

Ok, but it still have a mix of useVsPreviewPool: true and useVsPreviewPool: false. I think it is way better to have one toolset for the whole build and a warning than it is to deal with combinations of compilers in the build.

Why is it so important to fix this warning?

@safern
Copy link
Copy Markdown
Member Author

safern commented Feb 26, 2020

Yes the reason for the mix is because coreclr fails with the Windows SDK that the VS2019 preview when building the test components. So I’ll need to figure out why and then we can build all partitions with the same pool, this is temporary.

The reason why it is important to have this warnings be errors on CI is because locally we have warnings as errors and for nullable annotations and analyzer rules it has happened that some warnings have been introduced on a PR causing local development to fail with errors.

@jkotas
Copy link
Copy Markdown
Member

jkotas commented Feb 26, 2020

Why is it not an option to just disable that specific warning, or disable the warnings for just the native portion of the build? I think that should be our default way to deal with any warnings-as-errors issues.

@ViktorHofer
Copy link
Copy Markdown
Member

/azp run runtime

@ViktorHofer
Copy link
Copy Markdown
Member

/azp run dotnet-runtime-perf

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

2 similar comments
@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@safern
Copy link
Copy Markdown
Member Author

safern commented Feb 26, 2020

Why is it not an option to just disable that specific warning, or disable the warnings for just the native portion of the build? I think that should be our default way to deal with any warnings-as-errors issues.

The problem is the way we do the native build in libraries at the moment. The native build is driven by MSBuild, we call Exec task as an in proc to our build script under libraries/Native. Since warnaserror is controlled globally by arcade passing down the /warnaserror flag to MSBuild and the /p:TreatErrorsAsWarnings=true for the tasks at the moment we can only either disable warn as error for all, the managed and native or enable it. So I think it is better to just use a slightly different pool for a few weeks rather than causing pain to devs because of the state we’re in. This is currently blocking official builds so I would say we should merge this and then invest in moving coreclr, mono and installer to use the 2019 preview pools.

@jkotas
Copy link
Copy Markdown
Member

jkotas commented Feb 27, 2020

we call Exec task as an in proc to our build script under libraries/Native

I believe that the build-native.cmd script is executed out of proc: https://github.com/dotnet/runtime/blob/master/src/libraries/Native/build-native.proj#L47. It should be easy to tweak the warning as errors behavior for this invocation. I believe that changing https://github.com/dotnet/runtime/blob/master/src/libraries/Native/build-native.cmd#L150 to:

set __msbuildArgs=/p:Platform=%__BuildArch% /p:PlatformToolset="%__PlatformToolset%" -noWarn:MSB8065

should silence the warning/error.

@safern
Copy link
Copy Markdown
Member Author

safern commented Feb 27, 2020

Hmm I didn’t know we could do that in native as well. I’ll try that, thanks that’s a good suggestion.

@safern safern changed the title Use VS2019 Preview for libraries official build Disable force_install warning produced by CMake for libraries native build. Feb 27, 2020
:BuildNativeProj
:: Build the project created by Cmake
set __msbuildArgs=/p:Platform=%__BuildArch% /p:PlatformToolset="%__PlatformToolset%"
set __msbuildArgs=/p:Platform=%__BuildArch% /p:PlatformToolset="%__PlatformToolset%" -noWarn:MSB8065
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: Link to the issue to get this workaround removed would be nice

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks. We don’t have an issue open to update the toolset in the build pools. I’m going to merge and once I’m back from vacation will follow up on this matter and add the issue link if needed.

@safern safern merged commit b52c779 into master Feb 27, 2020
@safern safern deleted the Vs2019PreviewOfficialBuild branch February 27, 2020 14:05
@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Build errors: install_force has not been created.

4 participants