Build NuGet.Client even if response file is detected #3623
Conversation
|
Given this PR depends on #3552, I'm marking this PR as draft to signal that it's not as high priority to review just yet. I spoke with @heng-liu about that PR, and it's a little bit problematic because of net5 breaking changes and CI agents not having preview VS, but we think we have a path forward before VS 16.8 goes GA. Please mark the PR as ready to review once #3552 is merged and you rebase your changes. |
5bbe969 to
dd93578
Compare
dd93578 to
eb64f0f
Compare
2d2480e to
9ad4cb0
Compare
d4edea3 to
7d1f203
Compare
|
@Nirmal4G PR #3552 was closed, because of CI infrastructure issues. Basically, we can't implement those changes until VS 16.8 goes GA, and our CI machines are updated to that version. So, we should be able to in a few weeks, but not yet. If you can modify your branch so that it does not depend on the changes in #3552, then we can consider taking your changes. Thanks. |
7d1f203 to
d4b0a73
Compare
d4b0a73 to
a1228ec
Compare
There was a problem hiding this comment.
The Windows jobs (unit tests and func tests) failed the "Install .NET 5.0 for build" step:
. 'C:\A\1\71\s\scripts\utils\InstallCLIforBuild.ps1' 5.0.100-rc.1.20452.10
Downloading .NET SDK Install Script
Channel is : 5.0.100-rc.1.20452.10 latest
##[error]Invoke-RestMethod : BlobNotFoundThe specified blob does not exist.
RequestId:ce54500d-501e-007f-035a-9a36f9000000
Time:2020-10-04T14:29:41.3924590Z
At C:\A\1\71\s\scripts\utils\InstallCLIforBuild.ps1:47 char:20
+ $versionFile = Invoke-RestMethod -Method Get -Uri $httpGetUrl
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ CategoryInfo : InvalidOperation: (System.Net.HttpWebRequest:HttpWebRequest) [Invoke-RestMethod], WebExc
eption
+ FullyQualifiedErrorId : WebCmdletWebResponseException,Microsoft.PowerShell.Commands.InvokeRestMethodCommand
The version of SDK should be installed is :
Probing folder : C:\A\1\_temp\dotnet\sdk\
C:\A\1\_temp\dotnet\dotnet-install.ps1 5.0.100-rc.1.20452.10 latest
##[error]Could not resolve version information.
At C:\A\1\_temp\dotnet\dotnet-install.ps1:323 char:9
+ throw "Could not resolve version information."
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ CategoryInfo : OperationStopped: (Could not resolve version information.:String) [], RuntimeException
+ FullyQualifiedErrorId : Could not resolve version information.
##[error]Process completed with exit code 1 and had 2 error(s) written to the error stream.
Finishing: Install .NET 5.0 for build
On Linux, the "Install .NET 5.0 for build" step was incorrectly reported as successful, but when I look at the logs I see:
5.0.100-rc.1.20452.10
Channel is: 5.0.100-rc.1.20452.10 Version is: latest
Installing dotnet CLI into /home/vsts/work/_temp folder for building
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0
0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0
0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0
100 34939 100 34939 0 0 109k 0 --:--:-- --:--:-- --:--:-- 109k
curl: (22) The requested URL returned error: 404 The specified blob does not exist.
Add /home/vsts/work/_temp/dotnet to PATH
/home/linuxbrew/.linuxbrew/bin:/home/linuxbrew/.linuxbrew/sbin:/usr/share/rust/.cargo/bin:/home/runner/.config/composer/vendor/bin:/home/runner/.dotnet/tools:/snap/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin:/home/vsts/work/_temp/dotnet
Deleting .NET Core temporary files
A compatible installed .NET Core SDK for global.json version [5.0.100-preview] from [/home/vsts/work/1/s/global.json] was not found
The CliVersionForBuilding property in config.props isn't used in build.ps1, so a successful local build won't test this. Locally, our scripts assume that the dev has installed an appropriate version themselves, and we use the system dotnet cli. On CI, we know the agents contain only GA versions of the dotnet sdk, so we have additional steps that only run on CI to get the preview SDK.
5dd0db7 to
2a92648
Compare
|
35 functional tests on Windows failed. All of them were Dotnet.Integration.Tests PackCommandTests. I'm sure they all failed for the same reason. One of them failed with this message: with this stack trace: Both Linux and Mac failed to even build the solution. The "Install .NET 5 for build" step on both have the same error message. They both execute |
|
@Nirmal4G I rebased your PR so it includes some test fixes. Other people's PR builds are green now, but your branch has two errors. On Mac, the log has this output: I'm not 100% sure it's related to your changes, but I've also never before seen xunit discover the tests, but then not actually execute any of them. On Linux, tests run, but 17 This class, As a sanity check, it makes sure that the test SDK only has a single folder in the SDK subdirectory, but for some reason there's more than one. Frankly, I have no guesses why this is happening, but this happened for at least two builds of this branch, and other people's branches are passing, so it appears related to the changes in this PR. |
ed51f6f to
20ef07c
Compare
Adding "eol" to git attributes file for BASH script files to always checkout with LF line endings
Fail early if the external commands encounter an error
MSBuild picks up response files automatically, which messes up reading from standard output. This fixes the way it's read from std output, so that it always gets the intended value. Also, this is just a simplified hack and in no-way it can get the right value from any MSBuild target output stream.
The current logic assumes there will be no multi-value dotnet testing branches but we could be wrong... So, to be on the safe side, It's better to match the logic with functional tests' version.
Since the test frameworks use copied and patched cli, mismatch can cause tests fail. One of the failures caused when sometimes "NuGetFallbackFolder" becomes the first directory to resolve "dotnet.dll" against. It is now fixed by excluding the fallback folder in the resolving logic.
04c290b to
79fb279
Compare
|
Now only tests on mac are failing. Here's the full build logs: Looks like I was wrong about running tests using the system dotnet. At At I just remembered that months/years ago, I read something saying that Apple had a very old veresion of Bash on OSX, and they also changed the default shell to zsh. I don't know if our CI builds on Mac are using zsh, or bash, and if it's using bash, what version. Although you might be able to figure it out from the build logs. If your changes to Like you, I don't have access to a Mac, at least, not until we're allowed back in the office. So, we're at the mercy of someone with a Mac testing for us. Alternatively, you could do more research into what shell Azure DevOps runs scripts on, then run the same shell in your linux/wsl install, and see if the script fails like on CI, allowing you to test locally. Another option is to undo all your changes and just create an empty response file in the repo root. I don't know how a response file helps you manage your builds/repos, but creating an empty response file in NuGet.Client's root should prevent msbuild finding the response file outside the repo root, and be a lower complexity solution to the problem. |
There was a problem hiding this comment.
Another option is to undo all your changes and just create an empty response file in the repo root.
This doesn't work. It doesn't matter where the response file is but as long as there's one that MSBuild can find and load, it displays that message and fails to get the version from the GetCliBranchForTesting target.
There should be a better way to get values from MSBuild targets.
Bash on macOS is v3.2 but only v4.3+ supports negative indexing. This caused the script skipping the .NET SDK install for to be used with .NET CLI integration tests, which in-turn caused the build to fail. This is a verified workaround that works on Bash versions < v4.3
0744a8a to
2070d4e
Compare
What output? When I tested myself I didn't see any extra output, and looking at MSBuild's source code, I only see extra output at diagnostic verbosity, not minimal verbosity: https://github.com/dotnet/msbuild/blob/1629921c2b537d4452ff238981bd18519b8f5230/src/MSBuild/XMake.cs#L2316-L2319 |
|
I looked at the source too. I don't know why or how but this is how, it outputs for me…! nuget-client > msbuild build\config.props -nologo -v:q -t:GetCliBranchForTesting
Some command line switches were read from the auto-response file "MSBuild.rsp". To disable this file, use the "-noAutoResponse" switch.
D:\Apps\Tools\MSBuild\Current\Bin\msbuild.exe -nologo -bl -t:GetCliBranchForTesting -v:q build\config.propsnuget-client > msbuild build\config.props -nologo -v:m -t:GetCliBranchForTesting
Some command line switches were read from the auto-response file "MSBuild.rsp". To disable this file, use the "-noAutoResponse" switch.
D:\Apps\Tools\MSBuild\Current\Bin\msbuild.exe -nologo -bl -t:GetCliBranchForTesting -v:m build\config.props
release/5.0.2xx 5.0.200-servicing.21120.4nuget-client > msbuild build\config.props -nologo -v:n -t:GetCliBranchForTesting
Some command line switches were read from the auto-response file "MSBuild.rsp". To disable this file, use the "-noAutoResponse" switch.
D:\Apps\Tools\MSBuild\Current\Bin\msbuild.exe -nologo -bl -t:GetCliBranchForTesting -v:n build\config.props
Build started 31-03-2021 12:06:45 AM.
Project "E:\Projects\Microsoft.NET\nuget-client\build\config.props" on node 1 (GetCliBranchForTesting target(s)).
GetCliBranchForTesting:
release/5.0.2xx 5.0.200-servicing.21120.4
Done Building Project "E:\Projects\Microsoft.NET\nuget-client\build\config.props" (GetCliBranchForTesting target(s)).
•••
Build succeeded.
0 Warning(s)
0 Error(s)
Time Elapsed 00:00:00.09 |
|
Finally, All ✔️. 🥳 |
Bug
Fixes: NuGet/Home#9973
Fixes: NuGet/Home#10139
Regression: No
Fix
Details:
Checkout bash script files with Unix line ending (only LF) even on Windows. This makes sure that we can use the repo in WSL also.
MSBuild picks up response files automatically, which messes up the version logic reading from standard output. Adding
-noAutoRspto it is one solution but you have to add it to every MSBuild call to getuniform behavior. So, I have refactored the logic to work without needing it.
Also,
build.shcurrently downloads dotnet cli v1.0 for bootstrapping and runs MSBuild to get the dotnet cli version to build and test against. Since, v1 is out of support, the Sdk is not available to download. Wehave to update the version to 2.2, similar to what is specified in
runFuncTests.sh. Because of the nature of the change in the POSIX scripts, we can no longer depend onspacecharacter for splitting the branchand version string in the
CliBranchForTestingproperty inbuild\config.props. Instead, I have used colon:as a separator.For Example:
NOTE: This patch was separated from #3270.
Testing/Validation
Tests Added: No
Reason for not adding tests: Build Infra changes only
Validation: Through successful CI