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

Honor RecursiveDir when used on packed items in framework packs #34291

Merged
merged 3 commits into from
Jul 13, 2021

Conversation

ericstj
Copy link
Member

@ericstj ericstj commented Jul 12, 2021

When packaging Analyzers in the ref-pack we were setting PackagePath to the full path inside the package (by appending %(RecursiveDir)) then Nuget was appending %(RecursiveDir) again during pack. Avoid this by removing the expansion in our own items and fixing the CreateFrameworkList task to append %(RecurisveDir) in a similar manner to NuGet.

Addresses #34272

@ericstj ericstj requested review from Pilchie and a team as code owners July 12, 2021 19:35
@captainsafia
Copy link
Member

Thanks for digging into this. Let's try undoing the workarounds made in the dependency update PR to see if this works. I can undo the changes made in that PR if you add me as a collaborate to your fork, @ericstj.

@ericstj
Copy link
Member Author

ericstj commented Jul 12, 2021

I've added you, @captainsafia

@captainsafia
Copy link
Member

Looks like there is still a failure related to the logging generator:

CSC(0,0): error CS0006: (NETCORE_ENGINEERING_TELEMETRY=Build) Metadata file 'D:\workspace\_work\1\s\.dotnet\packs\Microsoft.AspNetCore.App.Ref\6.0.0-ci\analyzers/dotnet/cs/Microsoft.Extensions.Logging.Generators.dll' could not be found

The error occurs when building the InteropWebsite project on Helix.

@ericstj
Copy link
Member Author

ericstj commented Jul 12, 2021

It looks like it's running from packs copied to the repo local .dotnet/packs folder. I suspect there is part of AspNet infra that's copying things to that location. It must be copying the framework list but not the rest of the ref pack.

@ericstj
Copy link
Member Author

ericstj commented Jul 12, 2021

I see, these are additional targets in the ref-pack project.

<Target Name="_BatchCopyToLayoutTargetDir"
DependsOnTargets="_ResolveTargetingPackContent"
Inputs="@(RefPackContent)"
Outputs="@(RefPackContent->'$(LayoutTargetDir)%(PackagePath)%(FileName)%(Extension)')">
<Copy SourceFiles="@(RefPackContent)"
DestinationFiles="@(RefPackContent->'$(LayoutTargetDir)%(PackagePath)%(FileName)%(Extension)')"
UseHardlinksIfPossible="true" />
<Message Importance="High" Text="$(MSBuildProjectName) -> $(LayoutTargetDir)" />
</Target>
<ItemGroup>
<CreateDirectory Include="$(LocalInstallationOutputPath)" />
</ItemGroup>
<!-- Workaround https:/dotnet/sdk/issues/2910 by copying targeting pack into local installation. -->
<Target Name="_InstallTargetingPackIntoLocalDotNet"
DependsOnTargets="_ResolveTargetingPackContent"
Inputs="@(RefPackContent)"
Outputs="@(RefPackContent->'$(LocalInstallationOutputPath)%(PackagePath)%(FileName)%(Extension)')">
<Copy SourceFiles="@(RefPackContent)"
DestinationFiles="@(RefPackContent->'$(LocalInstallationOutputPath)%(PackagePath)%(FileName)%(Extension)')"
UseHardlinksIfPossible="true" />
<Message Importance="High" Text="$(MSBuildProjectName) -> $(LocalInstallationOutputPath)" />
</Target>

@@ -125,5 +125,17 @@ public override bool Execute()

return !Log.HasLoggedErrors;
}
private static string GetPackagePath(ITaskItem item)
Copy link
Member

Choose a reason for hiding this comment

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

nit:

Suggested change
private static string GetPackagePath(ITaskItem item)
private static string GetPackagePath(ITaskItem item)

@captainsafia
Copy link
Member

Merging to unblock SDK insertion.

@captainsafia captainsafia merged commit bc6355d into dotnet:main Jul 13, 2021
@ghost ghost added this to the 6.0-preview7 milestone Jul 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants