-
-
Notifications
You must be signed in to change notification settings - Fork 970
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
Create empty Directory.Build.props to ignore other prop files #2467
base: master
Are you sure you want to change the base?
Conversation
Perhaps we could adjust the Old: <Project Sdk="$SDKNAME$">
<PropertyGroup>
<ImportDirectoryBuildProps>false</ImportDirectoryBuildProps>
<ImportDirectoryBuildTargets>false</ImportDirectoryBuildTargets>
</PropertyGroup>
</Project> New: <Project>
<PropertyGroup>
<ImportDirectoryBuildProps>false</ImportDirectoryBuildProps>
<ImportDirectoryBuildTargets>false</ImportDirectoryBuildTargets>
</PropertyGroup>
<Sdk Name="$SDKNAME$">
</Project> (I forgot where I read that that could work.) [Edit] Sources: So, I'm not entirely sure it will work with <Project>
<PropertyGroup>
<ImportDirectoryBuildProps>false</ImportDirectoryBuildProps>
<ImportDirectoryBuildTargets>false</ImportDirectoryBuildTargets>
</PropertyGroup>
<Import Project="Sdk.props" Sdk="$SDKNAME$" />
<Import Project="Sdk.targets" Sdk="$SDKNAME$" />
</Project> |
The solution that I am proposing here was recommended in #2371 (comment) by @ViktorHofer who is the MSBuild expert on the .NET Team. It just works. |
Yeah, you could defer the SDK import but that would make the project file look very different. The Directory.Build.props solution is my preferred approach. |
That was addressed in #2393. The cli arguments override whatever is in the props, even if we don't ignore it. I'm not sure if other properties are consequential, but this looks good to me otherwise. |
The directory props are imported before the project, so there it would be importing 2 props files, no? Wouldn't we want to ignore the directory props in the case of wasm also? Also, the NativeAot generator needs this as well. |
Context: #2371 (comment)
It should not affect the WASM benchmarks (cc @radical), as they have their own generator:
BenchmarkDotNet/src/BenchmarkDotNet/Toolchains/MonoWasm/WasmGenerator.cs
Line 52 in 630622b
BenchmarkDotNet/src/BenchmarkDotNet/Templates/WasmCsProj.txt
Lines 3 to 5 in 630622b
which links a dedicated props file: https://github.com/dotnet/performance/blob/main/src/benchmarks/micro/MicroBenchmarks.Wasm.props
But I am not sure about dotnet/performance in general, as IIRC the props file there alters some settings like build output path. This is going to require a detailed testing (@cincuranet @LoopedBard3 @DrewScoggins @caaavik-msft @sblom)
fixes #2371