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

Use Microsoft.IO.Redist in XMake.cs #10705

Merged
merged 4 commits into from
Oct 9, 2024

Conversation

rainersigwald
Copy link
Member

This will fix #10540--it looks like M.IO.Redist has the same fix that .NET 9 has that avoids the failed root enumeration. As a bonus it's generally higher performance/less allocatey (but I don't know that that has a specific positive impact in this file).

This will fix dotnet#10540--it looks like M.IO.Redist has the same fix that
.NET 9 has that avoids the failed root enumeration. As a bonus it's
generally higher performance/less allocatey (but I don't know that that
has a specific positive impact in this file).
@rainersigwald
Copy link
Member Author

I'm going to solve the test failure in Microsoft.Build.UnitTests.CommandLineSwitchesTests.ProcessProfileEvaluationInvalidFilename by deleting the test. It only worked on .NET Framework, so the new behavior is parity with dotnet msbuild, and the functionality is rarely used.

I did, however, fix the error reporting at the end of the build to match the (better) core behavior:

dotnet msbuild /profileevaluation:"|||" .\src\Framework\ -t:asdf
  
Microsoft.Build.Framework failed with 1 error(s) (0.0s)
    
Q:\src\msbuild\src\Framework\Microsoft.Build.Framework.csproj : error MSB4057: The target "asdf" does not exist in the project.


Build failed with 1 error(s) in 0.4s
Writing profiler report to '|||'...
MSBUILD : error MSB4239: Error writing profiling report. The filename, directory name, or volume label syntax is incorrect. : 'Q:\src\msbuild\|||'.msbuild /profileevaluation:"|||" .\src\Framework\ -t:asdf

MSBuild version 17.13.0-preview-24504-04+c4d51a11b for .NET Framework

MSBUILD : error MSB1053: Provided filename is not valid. Illegal characters in path.
Switch: |||

For switch syntax, type "MSBuild -help"

artifacts\bin\bootstrap\net472\MSBuild\Current\Bin\amd64\MSBuild.exe /profileevaluation:"|||" .\src\Framework\ -t:asdf
MSBuild version 17.12.0-dev-24507-01+71d7380fc for .NET Framework


Q:\src\msbuild\src\Framework\Microsoft.Build.Framework.csproj : error MSB4057: The target "asdf" does not exist in the project.

Build FAILED.


Q:\src\msbuild\src\Framework\Microsoft.Build.Framework.csproj : error MSB4057: The target "asdf" does not exist in the project.

    0 Warning(s)

    1 Error(s)


Time Elapsed 00:00:00.93
Writing profiler report to '|||'...
MSBUILD : error MSB4239: Error writing profiling report. Illegal characters in path.

This was repeated but can be simplified.
This was thrown from GetExtension with an invalid path on .NET Framework.
This tested behavior of System.IO that only appled on
.NET Framework and was removed by dotnet#10705. Since evaluation
profiling is rarely used, I prefer to just drop the test.
@rainersigwald
Copy link
Member Author

@JanKrivanek @maridematte I changed this since you reviewed, can you give it a once-over and holler if you hate it?

@JanKrivanek
Copy link
Member

I'm all good with the changes here (including the removed unit test) - feel free to merge!

@rainersigwald rainersigwald merged commit 94941d9 into dotnet:main Oct 9, 2024
10 checks passed
@rainersigwald rainersigwald deleted the mio-redist-in-main branch October 9, 2024 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: can't find project file in root directory
4 participants