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

[wasm] Build tasks for net472 also #51959

Merged
merged 4 commits into from
Apr 28, 2021
Merged

Conversation

radical
Copy link
Member

@radical radical commented Apr 27, 2021

  • And update the tasks to be buildable with net472
    • avoid newer APIs
    • can't use init, and ranges
    • NotNull attribute isn't available
    • Path.GetRelativePath isn't available, but that is only used by
      Utils.GetDirectory, which isn't needed for wasm

radical added 2 commits April 27, 2021 18:10
- And update the tasks to be buildable with net472
    - avoid newer APIs
    - can't use `init`, and ranges
    - `NotNull` attribute isn't available
    - `Path.GetRelativePath` isn't available, but that is only used by
      `Utils.GetDirectory`, which isn't needed for wasm
radical added 2 commits April 27, 2021 19:24
On lewing's suggestion:

- add a dummy `NotNullAttribute` for net472 case
- and disable nullable warnings for that
    - any nullable issues should get checked in the net6.0 case anyway
Copy link
Member

@lewing lewing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code changes look fine. How confident are you about the publish stuff?

@radical
Copy link
Member Author

radical commented Apr 28, 2021

Not 100%. I want to see if we break any tests. And we should try out the packages.

@lewing lewing requested review from steveisok and akoeplinger April 28, 2021 18:02
@radical radical marked this pull request as ready for review April 28, 2021 20:57
@radical radical requested a review from marek-safar as a code owner April 28, 2021 20:57
Copy link
Member

@steveisok steveisok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@radical radical merged commit 85e4aea into dotnet:main Apr 28, 2021
@radical radical deleted the wasm-tasks-net472 branch April 28, 2021 21:26
lewing pushed a commit to lewing/runtime that referenced this pull request Apr 29, 2021
mmitche pushed a commit that referenced this pull request May 3, 2021
…io (#52078)

* [wasm] Build tasks for net472 also (#51959)

* [wasm] Fix loading WebAssembly tasks in VS

- In case of `WasmAppBuilder.dll`, we were packaging only the task assembly, and
  `System.Reflection.MetadataLoadContext` for net472, same as net6.0 .

- But for net472, VS fails with errors like:

```
C:\Program Files\dotnet\packs\Microsoft.NET.Runtime.WebAssembly.Sdk\6.0.0-preview.4.21222.10\Sdk\WasmApp.targets(424,4): Error MSB4018: The "PInvokeTableGenerator" task failed unexpectedly.
System.IO.FileNotFoundException: Could not load file or assembly 'System.Reflection.Metadata, Version=1.4.5.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a' or one of its dependencies. The system cannot find the file specified.
File name: 'System.Reflection.Metadata, Version=1.4.5.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a'
   at System.Reflection.MetadataLoadContext.LoadFromStreamCore(Stream peStream)
   at System.Reflection.MetadataLoadContext.LoadFromAssemblyPath(String assemblyPath)
   at System.Reflection.PathAssemblyResolver.Resolve(MetadataLoadContext context, AssemblyName assemblyName)
   at System.Reflection.MetadataLoadContext.TryFindAssemblyByCallingResolveHandler(RoAssemblyName refName)
   at System.Reflection.MetadataLoadContext.ResolveToAssemblyOrExceptionAssembly(RoAssemblyName refName)
   at System.Reflection.MetadataLoadContext.TryResolveAssembly(RoAssemblyName refName, Exception& e)
   at System.Reflection.MetadataLoadContext.TryGetCoreAssembly(String coreAssemblyName, Exception& e)
   at System.Reflection.TypeLoading.CoreTypes..ctor(MetadataLoadContext loader, String coreAssemblyName)
   at System.Reflection.MetadataLoadContext..ctor(MetadataAssemblyResolver resolver, String coreAssemblyName)
   at PInvokeTableGenerator.GenPInvokeTable(String[] pinvokeModules, String[] assemblies) in /Users/radical/dev/r2/src/tasks/WasmAppBuilder/PInvokeTableGenerator.cs:line 42
   at PInvokeTableGenerator.Execute() in /Users/radical/dev/r2/src/tasks/WasmAppBuilder/PInvokeTableGenerator.cs:line 28
   at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute()
   at Microsoft.Build.BackEnd.TaskBuilder.<ExecuteInstantiatedTask>d__26.MoveNext()
```

- So, bundle all the dependent assemblies from the publish folder

* Address review feedback

* Update src/tasks/AotCompilerTask/MonoAOTCompiler.csproj

Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>

* Update src/tasks/AotCompilerTask/MonoAOTCompiler.csproj

Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>

* Update src/tasks/WasmAppBuilder/WasmAppBuilder.csproj

Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>

* Update src/tasks/WasmAppBuilder/WasmAppBuilder.csproj

Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>

* Apply suggestions from code review

Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>

* Use a property for net472

Co-authored-by: Ankit Jain <radical@gmail.com>
Co-authored-by: Daniel Plaisted <dsplaisted@gmail.com>
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jul 8, 2021
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