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

Breaking change in response file parsing #26026

Closed
matt-richardson opened this issue Jun 15, 2022 · 18 comments
Closed

Breaking change in response file parsing #26026

matt-richardson opened this issue Jun 15, 2022 · 18 comments
Assignees
Labels
Area-CLI Bug cli-ux Issues and PRs that deal with the UX of the CLI (exit codes, log output, verbs/options, and so on)
Milestone

Comments

@matt-richardson
Copy link

Describe the bug

Since upgrading from SDK 6.0.300 to 6.0.301, the response file parsing logic has changed, where

-p:VSTestTestAdapterPath=".;C:\BuildAgent\plugins\dotnet\tools\vstest15"

used to work, but now shows an error:

dotnet build .\IPFX.UC.PresenceServer.sln @C:\BuildAgent\temp\agentTmp\1.rsp
Microsoft (R) Build Engine version 17.2.0+41abc5629 for .NET
Copyright (C) Microsoft Corporation. All rights reserved.
MSBUILD : error MSB1006: Property is not valid.
Switch: C:\BuildAgent\plugins\dotnet\tools\vstest15
For switch syntax, type "MSBuild -help"

To Reproduce

docker run -it --rm mcr.microsoft.com/dotnet/sdk:6.0.301 bash
mkdir repro
cd repro
dotnet new console
echo "-p:VSTestTestAdapterPath=\".;/opt/buildagent/plugins/dotnet/tools/vstest15\"" > params.rsp
dotnet restore repro.csproj @params.rsp

Exceptions (if any)

MSBUILD : error MSB1006: Property is not valid.
Switch: /opt/buildagent/plugins/dotnet/tools/vstest15

For switch syntax, type "MSBuild -help"

Expected

  Determining projects to restore...
  All projects are up-to-date for restore.

Further technical details

  • Include the output of dotnet --info
# dotnet --info
.NET SDK (reflecting any global.json):
 Version:   6.0.301
 Commit:    43f9b18481

Runtime Environment:
 OS Name:     debian
 OS Version:  11
 OS Platform: Linux
 RID:         debian.11-arm64
 Base Path:   /usr/share/dotnet/sdk/6.0.301/

Host (useful for support):
  Version: 6.0.6
  Commit:  7cca709db2

.NET SDKs installed:
  6.0.301 [/usr/share/dotnet/sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 6.0.6 [/usr/share/dotnet/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 6.0.6 [/usr/share/dotnet/shared/Microsoft.NETCore.App]

To install additional .NET runtimes or SDKs:
  https://aka.ms/dotnet-download
  • The IDE (VS / VS Code/ VS4Mac) you're running on, and its version
    N/A
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@alvaromarithompson
Copy link

TeamCity have addressed this issue with a solution found at JetBrains/teamcity-dotnet-plugin#158 (comment)

@LinardAtTenForce
Copy link

Note that this isn't only a issue with that specific parameter or with semi-colons in strings alone.

Another .rsp reproduction:

-p:teamcity_buildConfName="Build, Test and Publish"

Error:

Starting: .NET SDK 6.0.301 "C:\Program Files\dotnet\dotnet.exe" test [...].csproj @[...].rsp
MSBUILD : error MSB1006: Property is not valid.
Switch: Test and Publish
For switch syntax, type "MSBuild -help"

@baronfel
Copy link
Member

baronfel commented Jun 16, 2022

My suspicion is that the response file features of system.commandline are covering the MSBuild response file parsing, and so we need to do some work to disable/sidestep those features.

I haven't been able to verify that this branch contains a version of system.commandline with this feature, yet.

UPDATE: the changes I'm thinking of didn't arrive until beta4 of S.CL, which isn't in the 6.0.3xx branch of the SDK. It may be an actual change in MSBuild instead.

@loop-evgeny
Copy link

loop-evgeny commented Jun 16, 2022

This breaks our TeamCity builds badly - same issue as LinardAtTenForce. We have a comma in our build configuration name, so even after updating the plugin it failed due to that and we had to rename the build configuration, too.

@rbhanda
Copy link
Contributor

rbhanda commented Jun 16, 2022

@marcpopMSFT @dsplaisted @joeloff FYI. May be they can help here

@janv8000
Copy link

dotnet/msbuild#471 related?

@Danielku15
Copy link
Contributor

Cross posting from here as many people are seeing this combination with TeamCity.

A hint to the readers of this issue (which I missed first): the global.json does not need to be in the working/solution directory. You can also just place a global.json in your drive-root so that all builds on your server use this SDK version. This way we avoided updating all of our repositories and branches.

@dsplaisted
Copy link
Member

Ping @baronfel. Does this mean we should consider reverting the behavior change instead of just working around it in the Brotli task?

@baronfel
Copy link
Member

baronfel commented Jun 23, 2022

@janv8000 the MSBuild issue is not related. I must apologize for this - this issue is in fact related! The changes to the Option for MSBuild properties added a pathway where the MSBuild Property values weren't being semicolon-escaped as they are in other parts of the codebase. That was the second key part of the one/two punch of this regression.

@Danielku15 thank you for reporting a second case. @dsplaisted yes, I agree that a second case is enough to investigate reverting the S.CL update that triggered this, and then looking at how to mitigate for 6.0.400 and beyond.

@Danielku15
Copy link
Contributor

I think overall there are quite some cases in the market. They have been primarily reported to JetBrains as people using TeamCity (CI server) are directly affected with this issue after upgrading the SDK 😉 Our IT rolled out .net SDK updates last night and we started suffering of these errors.

If you need further cases you can refer to this Issue in the JetBrains issue tracker where a lot of people had the issue, but were able to patch their servers with some hotfix from JB:
https://youtrack.jetbrains.com/issue/TW-76527/Running-a-NET-builder-task-on-DotNet-6-SDK-solution-generates-a-build-error-MSBUILD-error-MSB1006-Property-is-not-valid

@baronfel
Copy link
Member

baronfel commented Aug 4, 2022

Closing as resolved and should be released in 6.0.303 and onwards.

@baronfel baronfel closed this as completed Aug 4, 2022
@baronfel baronfel added Bug cli-ux Issues and PRs that deal with the UX of the CLI (exit codes, log output, verbs/options, and so on) and removed untriaged Request triage from a team member labels Aug 4, 2022
@githubuser6000
Copy link

githubuser6000 commented Aug 15, 2022

Issue present in 6.0.400.

src$ dotnet publish -v detailed | grep -i "unrec"
         Unrecognized command or argument 'Files\Projects\projectdir\src\projectdir\bin\Debug\net6.0\wwwroot\_framework\blazor.webassembly.js'.
         Unrecognized command or argument 'Files\Projects\projectdir\src\projectdir\obj\Debug\net6.0\compress\5dWX0eyJ.br'.
         Unrecognized command or argument 'Files\Projects\projectdir\src\projectdir\obj\Debug\net6.0\linked\Microsoft.AspNetCore.Components.dll'.
         Unrecognized command or argument 'Files\Projects\projectdir\src\projectdir\obj\Debug\net6.0\compress\rSyiIJV8.br'.
 --- more lines ---
src$ dotnet --list-sdks
6.0.400 [C:\Program Files\dotnet\sdk]

Please don't get me wrong, but I have to say, MS quality testing is appalling, not just in windows but in all products these days? This is on a brand new project (dotnet new blazorwasm && dotnet publish -v detailed | grep -i "unrec"). I'd expect that at least to be tested. If this is the experience of someone testing the tech, doesn't bode well for the project.

@loop-evgeny
Copy link

Yes, easily reproducible on Linux, too, if the path contains a space:

~/src/with space$ dotnet new blazorwasm && dotnet publish -v detailed | grep -i "unrec"

Welcome to .NET 6.0!
---------------------
SDK Version: 6.0.400

...

The template "Blazor WebAssembly App" was created successfully.
This template contains technologies from parties other than Microsoft, see https://aka.ms/aspnetcore/6.0-third-party-notices for details.

Processing post-creation actions...
Running 'dotnet restore' on /home/em/src/with space/with space.csproj...
  Determining projects to restore...
  Restored /home/em/src/with space/with space.csproj (in 3 sec).
Restore succeeded.


         Unrecognized command or argument 'space/bin/Debug/net6.0/wwwroot/_framework/blazor.webassembly.js'.
...

@baronfel
Copy link
Member

Reopening this due to repro.

@baronfel baronfel added this to the 6.0.305 milestone Sep 13, 2022
@baronfel
Copy link
Member

Closing this as we merged the servicing fix last week. This should be fixed in the October servicing release of the 6.0.300 SDK series and up.

@matt-richardson
Copy link
Author

@baronfel - can you list the exact version(s) it's fixed in? "6.0.300 SDK series and up" nominally (logically?) includes 6.0.400, which exhibits the problem.

@baronfel
Copy link
Member

baronfel commented Sep 13, 2022

Good point, 6.0.305 and 6.0.402 should contain the fix. 'and up' was intended to mean 'the higher feature bands', which right now are 6.0.4xx and 7.0.1xx.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-CLI Bug cli-ux Issues and PRs that deal with the UX of the CLI (exit codes, log output, verbs/options, and so on)
Projects
None yet
Development

No branches or pull requests