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

MSBuildWorkspace does not cleanly load VB project on Linux or MacOS #72014

Open
JoeRobich opened this issue Feb 9, 2024 · 7 comments · May be fixed by #76888
Open

MSBuildWorkspace does not cleanly load VB project on Linux or MacOS #72014

JoeRobich opened this issue Feb 9, 2024 · 7 comments · May be fixed by #76888
Assignees
Milestone

Comments

@JoeRobich
Copy link
Member

Version Used: 4.10 Preview 1

Reported in #69225

Steps to Reproduce:

  1. dotnet new console --language VB
  2. use MSBuildWorkspace.OpenProjectAsync to load the newly created VB project

Expected Behavior:
Project loads without any workspace diagnostics and compilation reports no compiler diagnostics.

Actual Behavior:
Compilation reports "BC30002": Type 'Global.Microsoft.VisualBasic.ApplicationServices.ApplicationBase' is not defined.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Feb 9, 2024
@paul1956
Copy link
Contributor

paul1956 commented Feb 9, 2024

@JoeRobich I believe that is part of the WinForms VB Runtime not .Net Core, there have been some requests to move it and other parts of the VB Runtime down to Core since it would enable easier access from C# and allow VB code on Linux and Mac, but I have not seen any movement. There is very little in ApplicationBase (120 lines) but ApplicationBase.Info uses reflection on Windows to access Assembly and if you need that more work would be required.

@CyrusNajmabadi CyrusNajmabadi added the IDE-MSBuildWorkspace MSBuildWorkspace label Oct 25, 2024
@arunchndr arunchndr added Feature Request and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Nov 27, 2024
@arunchndr arunchndr added this to the Backlog milestone Nov 27, 2024
@arunchndr
Copy link
Member

Joey - please move this to the runtime repo when ready.

@JoeRobich
Copy link
Member Author

JoeRobich commented Nov 28, 2024

After doing a little more investigation, this isn't an issue for the Runtime or WinForms teams.

  1. The reported diagnostics are located in the "MyTemplateLocation", meaning the compiler provided template.
  2. The compiler doesn't report any diagnostics when performing a command line build.
  3. The binarylog of the command line build shows the _MyType should be defined as "Empty"
    Image
  4. The MyTemplate should not reference 'Global.Microsoft.VisualBasic.ApplicationServices.ApplicationBase' or any other type when _MyType is "Empty"
  5. The MSBuildWorkspace loaded project's ParseOptions show only 2 PreprocessorSymbol and neither are _MyType
  6. When _MyType is "" the template treats it as a Windows type which explains why it is referencing these classes that aren't available.

So why are we not getting the proper _MyType?

  1. The projectFileInfo.CommandLineArgs loaded from the ProjectFile in MSBuildProjectLoader.Worker shows many defined symbols but some odd character escaping.
  2. Checking the json read in RpcClient, the define looks like "/define:\"CONFIG=/\"Debug/\",DEBUG=-1,TRACE=-1,PLATFORM=/\"AnyCPU/\",NET=-1,NET9_0=-1,NETCOREAPP=-1,_MyType=/\"Empty/\",NET5_0_
  3. My type is defined but oddly escaped _MyType=/\"Empty/\"
  4. The CommandLineArgs come from reading the VbcCommandLineArgs Item from the design time build.
  5. Checking the VbcCommandLineArgs Item from a design-time build binlog, _MyList is report /"Empty/"
    Image
  6. Comparing this to the Vbc task's CommandLineArguments and I see the quotes properly escaped.
    Image
  7. This is happening on non-Windows platforms because MSBuild is "fixing up" the ItemSpec (replacing the Windows default path separator '\' with '/') when creating the VbcCommandLineArgs TaskItem.
  8. The JsonSerializer is then turns /"Empty/" into /\"Empty/\" when escaping the double quotes.
  9. Going back to the MSBuildProjectLoader.Worker and inspecting the parsed commandLineArgs.Errors there is indeed an error that is not surfaced.
error BC31030: Conditional compilation constant 'CONFIG= ^^ ^^ /"Debug/"' is not valid: Syntax error in conditional compilation expression.

Options:

  1. We could stop using the *CommandLineArgs Items as we already maintain *CommandLineArgumentReader classes which read build Properties (importantly not Items) to gather the same information.
  2. We could treat /" in the CommandLineArgs ItemSpec as an escaped double quote. However, there might be false positives.
  3. MSBuild could change the ItemSpec behavior. Likely the riskiest option at this point.

cc: @jasonmalinowski & @rainersigwald for thoughts

@jasonmalinowski
Copy link
Member

Is that TaskItem constructor being called here?

items[i] = new TaskItem(commandLineArgs[i]);

Because if so, it looks like there's another constructor that doesn't do escaping:

https://github.com/dotnet/msbuild/blob/4c6a5a963ceb38f77af4e57d28669872b616a8dc/src/Utilities/TaskItem.cs#L88-L96

Can we call that one instead?

@JoeRobich
Copy link
Member Author

Can we call that one instead?

Unfortunately it does call the other constructor which does the escaping. https://github.com/dotnet/msbuild/blob/4c6a5a963ceb38f77af4e57d28669872b616a8dc/src/Utilities/TaskItem.cs#L100

We could create out own ITaskItem implementation to wrap the args and pass it to this constructor. https://github.com/dotnet/msbuild/blob/4c6a5a963ceb38f77af4e57d28669872b616a8dc/src/Utilities/TaskItem.cs#L120-L123

Also, thank you for linking me to where this TaskItem was being created on our end.

@jasonmalinowski
Copy link
Member

@rainersigwald Any thoughts here? Invoking that copy constructor seems like a possible workaround.

@rainersigwald
Copy link
Member

rainersigwald commented Dec 4, 2024

. . . how on Earth is this the first time that unconditional correction has come up? We should definitely have an API to avoid it. (edit: dotnet/msbuild#11083)

I think you're right that the best workaround is a custom ITaskItem--I don't think you'll need to create a TaskItem from it though, you should be able to return your type IIRC.

JoeRobich added a commit that referenced this issue Jan 23, 2025
The TaskItem implementation from MSBuild treats the ItemSpec as file path and tries to normalize the path separators. When running on non-windows machines this means changing '\' to '/'.  However this breaks how double quotes are escaped in the compiler args. By using our own implemetation of TaskItem we can use the compiler args as the ItemSpec without any modification.

Resolves #72014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants