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

BlazorWasmSdk: Fix duplicate imports #32799

Merged
merged 1 commit into from
May 25, 2023
Merged

Conversation

radical
Copy link
Member

@radical radical commented May 25, 2023

Microsoft.NET.Sdk.BlazorWebAssembly.Current.targets unconditionally imports:

  <Import Sdk="Microsoft.NET.Sdk.Razor" Project="Sdk.targets" />
  <Import Sdk="Microsoft.NET.Sdk.Web.ProjectSystem" Project="Sdk.targets" />
  <Import Sdk="Microsoft.NET.Sdk.Publish" Project="Sdk.targets" />

.. after which it can import
Microsoft.NET.Sdk.BlazorWebAssembly.5_0.targets, which then unconditionally imports:

  <Import Sdk="Microsoft.NET.Sdk.Razor" Project="Sdk.targets" />
  <Import Sdk="Microsoft.NET.Sdk.Web.ProjectSystem" Project="Sdk.targets" />
  <Import Sdk="Microsoft.NET.Sdk.Publish" Project="Sdk.targets" />

.. which produces warning:

[2023/05/24 07:06:03][INFO] /home/helixbot/work/BAA509D0/p/dotnet/sdk/8.0.100-preview.5.23273.2/Sdks/Microsoft.NET.Sdk.BlazorWebAssembly/targets/Microsoft.NET.Sdk.BlazorWebAssembly.5_0.targets(13,3): warning MSB4011: "/home/helixbot/work/BAA509D0/p/dotnet/sdk/8.0.100-preview.5.23273.2/Sdks/Microsoft.NET.Sdk.Razor/Sdk/Sdk.targets" cannot be imported again. It was already imported at "/home/helixbot/work/BAA509D0/p/dotnet/sdk/8.0.100-preview.5.23273.2/Sdks/Microsoft.NET.Sdk.BlazorWebAssembly/targets/Microsoft.NET.Sdk.BlazorWebAssembly.Current.targets (25,3)". This is most likely a build authoring error. This subsequent import will be ignored.

.. and the same for Microsoft.NET.Sdk.Web.ProjectSystem, and Microsoft.NET.Sdk.Publish.

Since the 5_0.targets is only ever imported from the Current.targets, it does not need to import the same SDKs again.

This was introduced in:

commit 3e34299dff61129227b539b792973a7f7ad60d57
Author: Marek Fišera <mara@neptuo.com>
Date:   Tue Mar 28 15:47:55 2023 +0200

    Split WebAssembly SDK from Blazor SDK (#31154)

.. and the blazor scenario perf runs have been broken since it got merged.

`Microsoft.NET.Sdk.BlazorWebAssembly.Current.targets` unconditionally
imports:

```xml
  <Import Sdk="Microsoft.NET.Sdk.Razor" Project="Sdk.targets" />
  <Import Sdk="Microsoft.NET.Sdk.Web.ProjectSystem" Project="Sdk.targets" />
  <Import Sdk="Microsoft.NET.Sdk.Publish" Project="Sdk.targets" />
```

.. *after* which it can import
`Microsoft.NET.Sdk.BlazorWebAssembly.5_0.targets`, which then
unconditionally imports:

```xml
  <Import Sdk="Microsoft.NET.Sdk.Razor" Project="Sdk.targets" />
  <Import Sdk="Microsoft.NET.Sdk.Web.ProjectSystem" Project="Sdk.targets" />
  <Import Sdk="Microsoft.NET.Sdk.Publish" Project="Sdk.targets" />
```

.. which produces warning:

```
[2023/05/24 07:06:03][INFO] /home/helixbot/work/BAA509D0/p/dotnet/sdk/8.0.100-preview.5.23273.2/Sdks/Microsoft.NET.Sdk.BlazorWebAssembly/targets/Microsoft.NET.Sdk.BlazorWebAssembly.5_0.targets(13,3): warning MSB4011: "/home/helixbot/work/BAA509D0/p/dotnet/sdk/8.0.100-preview.5.23273.2/Sdks/Microsoft.NET.Sdk.Razor/Sdk/Sdk.targets" cannot be imported again. It was already imported at "/home/helixbot/work/BAA509D0/p/dotnet/sdk/8.0.100-preview.5.23273.2/Sdks/Microsoft.NET.Sdk.BlazorWebAssembly/targets/Microsoft.NET.Sdk.BlazorWebAssembly.Current.targets (25,3)". This is most likely a build authoring error. This subsequent import will be ignored.
```

.. and the same for `Microsoft.NET.Sdk.Web.ProjectSystem`, and `Microsoft.NET.Sdk.Publish`.

Since the `5_0.targets` is only ever imported from the
`Current.targets`, it does not need to import the same SDKs again.

This was introduced in:
```
commit 3e34299
Author: Marek Fišera <mara@neptuo.com>
Date:   Tue Mar 28 15:47:55 2023 +0200

    Split WebAssembly SDK from Blazor SDK (dotnet#31154)
```

.. and the blazor scenario perf runs have been broken since it got
merged.
@radical radical requested review from lewing and maraf May 25, 2023 01:50
@radical radical requested a review from a team as a code owner May 25, 2023 01:50
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-AspNetCore RazorSDK, BlazorWebAssemblySDK, dotnet-watch untriaged Request triage from a team member labels May 25, 2023
@ghost
Copy link

ghost commented May 25, 2023

Thanks for your PR, @radical.
To learn about the PR process and branching schedule of this repo, please take a look at the SDK PR Guide.

@maraf
Copy link
Member

maraf commented May 25, 2023

I can't find the warnings in CI logs, should we expose those some how?

@radical
Copy link
Member Author

radical commented May 25, 2023

I can't find the warnings in CI logs, should we expose those some how?

This is on the perf pipeline, and I plan to do that, but it needs a little more work.

@radical radical merged commit 6b89d6e into dotnet:main May 25, 2023
16 checks passed
@radical radical deleted the fix-blazor-perf-run branch May 25, 2023 15:48
@maraf
Copy link
Member

maraf commented May 25, 2023

This is on the perf pipeline, and I plan to do that, but it needs a little more work.

What I meant is that this repo has an test for blazor 5.0, we should see the warnings there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-AspNetCore RazorSDK, BlazorWebAssemblySDK, dotnet-watch untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants