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

Allow NoWarn on ProjectReference to supress ASPIRE004 #5230

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

CZEMacLeod
Copy link

@CZEMacLeod CZEMacLeod commented Aug 8, 2024

Description

Allow the suppression of the ASPIRE004 error message per ProjectReference using the NoWarn property.

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No
  • Does the change require an update in our Aspire docs?
    • Yes
      • Link to aspire-docs issue:
    • No
Microsoft Reviewers: Open in CodeFlow

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-integrations Issues pertaining to Aspire Integrations packages label Aug 8, 2024
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Aug 8, 2024
@eerhardt
Copy link
Member

eerhardt commented Aug 8, 2024

Can you describe the scenario where you would want to suppress this warning?

@CZEMacLeod
Copy link
Author

@eerhardt Yes - a project reference to a class library (or in my case an IIS Express SDK project) where you want the project information to be able to use it in a resource but it is not executable.

  <ItemGroup>
    <ProjectReference Include="..\..\..\src\C3D\Extensions\Aspire\IISExpress\C3D.Extensions.Aspire.IISExpress.csproj" IsAspireProjectResource="false" />
    <ProjectReference Include="..\..\..\src\C3D\Extensions\Aspire\WaitFor\C3D.Extensions.Aspire.WaitFor.csproj" IsAspireProjectResource="false" />
    <ProjectReference Include="..\EF6\EF6WebApp.csproj" NoWarn="ASPIRE004"/>
  </ItemGroup>

This is similar to suppressing nuget package warning etc.
You could do a global <NoWarn>ASPIRE004</NoWarn> (although I haven't tested that) - but then you might not notice when you add a library that should be IsAspireProjectResource="false".
Maybe an alternative could be if you explicitly do IsAspireProjectResource="true" then it wouldn't warn, but this matched the existing mechanism (which I tried first and found it didn't work here).

@eerhardt
Copy link
Member

eerhardt commented Aug 8, 2024

Maybe an alternative could be if you explicitly do IsAspireProjectResource="true" then it wouldn't warn

I think that approach would be better. I'm not sure the "NoWarn" on the ProjectReference is a gesture used anywhere else. So I wouldn't want to invent it here. Also the way the current PR is, it only supports a single "NoWarn", not a list.

Also, can you add a test for this scenario? Here is where the existing tests are:

/// <summary>
/// Tests that when an AppHost has a ProjectReference to a library project, a warning is emitted.
/// </summary>
[Fact]
public void EnsureWarningsAreEmittedWhenProjectReferencingLibraries()

@CZEMacLeod
Copy link
Author

CZEMacLeod commented Aug 9, 2024

@eerhardt NoWarn is metadata used on PackageReference and ProjectReference for nuget to suppress NU1701 etc. type errors. My issue is similar in style to NuGet/Home#5999

I'll have a look at the tests and see what I can do.
Yes - I did implement it only for a single error, rather than a list - but I felt a simple way of doing it was enough as there is only the one error you need to deal with just now. If additional warnings were added in the future, the condition could be updated.

I'm struggling with the running a local version of aspire just now, so it may be a while before I can get to dealing with the tests.
(And I've broken the aspire workload for all development!) I think I've ended up somewhere like #3691

dotnet workload restore

Installing workloads: aspire

Workload(s) 'aspire' are already installed.
Workload installation failed. Rolling back installed packs...
Workload installation failed: Failed to verify Authenticode signature, package: C:\ProgramData\dotnet\workloads\Aspire.Hosting.Sdk.Msi.x64\8.1.1-preview.1.24407.6\237134c84b4f4287a12f177ddc6df49f-x64.msi, allow online revocation checks: True Error: 0x800b0100, No signature was present in the subject.

This was after

.\eng\installLatestFromReleaseBranch.ps1 -FromMain

@CZEMacLeod
Copy link
Author

Oh and another area where this would be useful is rr-wfm/MSBuild.Sdk.SqlProj#585
They specifically mention the ASPIRE004 error there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-integrations Issues pertaining to Aspire Integrations packages community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants