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

Update solution files after projects changes #97969

Merged
merged 4 commits into from
Feb 5, 2024

Conversation

radekdoulik
Copy link
Member

@radekdoulik radekdoulik commented Feb 5, 2024

The project files have now different dependencies after changes in #97052

The project files have now diferent dependencies
@ghost
Copy link

ghost commented Feb 5, 2024

Tagging subscribers to this area: @dotnet/area-infrastructure-libraries
See info in area-owners.md if you want to be subscribed.

Issue Details

The project files have now different dependencies

Author: radekdoulik
Assignees: radekdoulik
Labels:

area-Infrastructure-libraries

Milestone: -

@radekdoulik radekdoulik changed the title Update solution files after changes in #97052 Update solution files after projects changes Feb 5, 2024
@radekdoulik
Copy link
Member Author

@ViktorHofer I used the slngen.proj to update these solution files. I noticed you updated solution files in the past too, do you know if that is the preferred way to do it?

@pavelsavara does the build work for you with these changes?

@ViktorHofer
Copy link
Member

I used the slngen.proj to update these solution files. I noticed you updated solution files in the past too, do you know if that is the preferred way to do it?

Yes. dotnet.cmd build src\libraries\slngen.proj. Did you invoke that command?

@pavelsavara
Copy link
Member

It's better/different problem now.

$env:WasmEnableThreads="true"
.\build.cmd -bl -os browser -subset mono+libs -c Debug
.\build.cmd -vs c:\Dev\runtime\src\libraries\System.Runtime.InteropServices.JavaScript\System.Runtime.InteropServices.JavaScript.sln

Build will pass.
Will open VS but would not work.
VS is somehow confused about it.

image

@pavelsavara
Copy link
Member

Severity	Code	Description	Project	File	Line	Suppression State
Error	
MSB4062	
The "CombineLinkerXmlFiles" task could not be loaded from the assembly 
C:\Dev\runtime\artifacts\bin\ILLink.Tasks\Debug\net472\ILLink.Tasks.dll. Could not load file or assembly 
'file:///C:\Dev\runtime\artifacts\bin\ILLink.Tasks\Debug\net472\ILLink.Tasks.dll' or one of its dependencies. The system cannot find the file specified. Confirm that the <UsingTask> declaration is correct, that the assembly and all its dependencies are available, and that the task contains a public class that implements Microsoft.Build.Framework.ITask.	System.Private.CoreLib (src\System.Private.CoreLib)	
C:\Dev\runtime\src\coreclr\System.Private.CoreLib\CreateRuntimeRootILLinkDescriptorFile.targets	52	

Note the error is pointing to CoreCLR files, but we need Mono.

@radekdoulik
Copy link
Member Author

I used the slngen.proj to update these solution files. I noticed you updated solution files in the past too, do you know if that is the preferred way to do it?

Yes. dotnet.cmd build src\libraries\slngen.proj. Did you invoke that command?

Yes, from different folder, so my cmd line was ../../../dotnet.sh build -t:UpdateSolutionFile ../slngen.proj and I used the UpdateSolutionFile target directly.

Then I picked only the solution files affected by the previous PR changes.

@ViktorHofer
Copy link
Member

Note the error is pointing to CoreCLR files, but we need Mono.

You will need to tell VS that. By default, CoreCLR is used. @akoeplinger do we already have an established pattern to link to the Mono's Private.CoreLib inside VS? I was suggesting to set $env:RuntimeFlavor=Mono / set RuntimeFlavor=Mono but maybe there's a better way?

The "CombineLinkerXmlFiles" task could not be loaded from the assembly
C:\Dev\runtime\artifacts\bin\ILLink.Tasks\Debug\net472\ILLink.Tasks.dll. Could not load file or assembly

This indicates that something tries to invoke the task before the project references are built. ILLink.Tasks.csproj should be a project reference dependency and automatically get built.

@ViktorHofer
Copy link
Member

Then I picked only the solution files affected by the previous PR changes.

We usually update all solution files but we can do that in a separate PR.

@akoeplinger
Copy link
Member

@akoeplinger do we already have an established pattern to link to the Mono's Private.CoreLib inside VS?

I'm not sure anyone tried, but setting RuntimeFlavor sounds like worth a try.

Though thinking about it, shouldn't VS compile against the ref SPC, not the implementation?

@radekdoulik
Copy link
Member Author

Though thinking about it, shouldn't VS compile against the ref SPC, not the implementation?

In this case we need the implementation, as it uses the ThrowOnBlockingWaitOnJSInteropThread field, which is not in the ref assembly.

@ViktorHofer
Copy link
Member

ViktorHofer commented Feb 5, 2024

Though thinking about it, shouldn't VS compile against the ref SPC, not the implementation?

Projects compile against ref/CoreLib but they (correctly) reference the src/CoreLib project which then references the ref/CoreLib.

I'm not sure anyone tried, but setting RuntimeFlavor sounds like worth a try.

We could respect that switch in build.ps1. #97975

@pavelsavara
Copy link
Member

$env:RuntimeFlavor=Mono

This helped with the ThrowOnBlockingWaitOnJSInteropThread but the problem with CombineLinkerXmlFiles from CoreCLR is still there

@pavelsavara
Copy link
Member

image

Actually, not really.

@ViktorHofer
Copy link
Member

I would hold off with merging this. I see the following on my VS when following the above steps:

image

@ViktorHofer
Copy link
Member

I reran slngen locally, opened the produced slns and the failure from the picture above is now gone. No idea what is different but I updated your branch with the file updates.

@pavelsavara
Copy link
Member

pavelsavara commented Feb 5, 2024

I'm good now, passing TargetArchitecture, RuntimeFlavor and TargetOS is now necessary, kind of logical.

$env:TargetArchitecture="wasm"
$env:RuntimeFlavor="Mono"
$env:TargetOS="browser"
.\build.cmd -vs c:\Dev\runtime\src\libraries\System.Runtime.InteropServices.JavaScript\System.Runtime.InteropServices.JavaScript.sln

Edit, it must be env variables not /p: because -vs is not passing those.

@radekdoulik radekdoulik marked this pull request as ready for review February 5, 2024 14:02
@radekdoulik radekdoulik requested a review from lewing as a code owner February 5, 2024 14:02
@ViktorHofer
Copy link
Member

Merging as slns aren't exercised in CI anyway.

@ViktorHofer
Copy link
Member

Follow-ups.

Filed #97984 to track Platform not being passed in correctly to System.Private.CoreLib's build.

Submitted #97982 to respect the -os and -arch build properties for the VS invocation.

Submitted #97974 to not cause task locks and to not trigger the illink tasks for CoreLib's build when opening VS and doing a design time build.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants