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

Fix clean and rebuild targets #33758

Merged
merged 1 commit into from
Mar 24, 2020
Merged

Fix clean and rebuild targets #33758

merged 1 commit into from
Mar 24, 2020

Conversation

Anipik
Copy link
Contributor

@Anipik Anipik commented Mar 19, 2020

Fixes #33721

The failure here was that we were automatically adding ProjectReference if the src builds for X tfm and we dont have corresponding ref.

And we were hitting the error because some of the src tfms dont have a comapatible configs in the ref csproj, rather they depend on restore to resolve those ref assemblies.

Clean target behavior:- it will run for all tfms of the source project

@ericstj
Copy link
Member

ericstj commented Mar 19, 2020

Looks like this broke all configurations build.

F:\workspace\_work\1\s\.packages\microsoft.dotnet.apicompat\5.0.0-beta.20168.1\build\Microsoft.DotNet.ApiCompat.targets(49,5): error : ResolvedMatchingContract item must be specified to run API compat. [F:\workspace\_work\1\s\src\libraries\Microsoft.Extensions.DependencyInjection\src\Microsoft.Extensions.DependencyInjection.csproj]
##[error].packages\microsoft.dotnet.apicompat\5.0.0-beta.20168.1\build\Microsoft.DotNet.ApiCompat.targets(49,5): error : ResolvedMatchingContract item must be specified to run API compat.

This is the first src project that builds that needs to run API compat. I believe this is because we don't binplace for every configuration.

@Anipik
Copy link
Contributor Author

Anipik commented Mar 19, 2020

The ref assembly for only this project Microsoft.Extensions.DependencyInjection.csproj is not getting built or something is deleting it

@ericstj
Copy link
Member

ericstj commented Mar 19, 2020

Hmm, sounds like you're right. This build is happening very late after ref/src during pkg build. I think this is because the pkgproj exists, but these projects are excluded from ref.builds/src.builds. The place to look for the problem is

<ItemGroup>
<PortedExtensionsProject Include="$(MSBuildThisFileDirectory)Microsoft.Extensions.Caching.Abstractions\**\*csproj" />
<PortedExtensionsProject Include="$(MSBuildThisFileDirectory)Microsoft.Extensions.Configuration\**\*csproj" />
<PortedExtensionsProject Include="$(MSBuildThisFileDirectory)Microsoft.Extensions.Configuration.Abstractions\**\*csproj" />
<PortedExtensionsProject Include="$(MSBuildThisFileDirectory)Microsoft.Extensions.Configuration.Binder\**\*csproj" />
<PortedExtensionsProject Include="$(MSBuildThisFileDirectory)Microsoft.Extensions.Configuration.CommandLine\**\*csproj" />
<PortedExtensionsProject Include="$(MSBuildThisFileDirectory)Microsoft.Extensions.Configuration.EnvironmentVariables\**\*csproj" />
<PortedExtensionsProject Include="$(MSBuildThisFileDirectory)Microsoft.Extensions.DependencyInjection.Abstractions\**\*csproj" />
<PortedExtensionsProject Include="$(MSBuildThisFileDirectory)Microsoft.Extensions.FileProviders.*\**\*csproj" />
<PortedExtensionsProject Include="$(MSBuildThisFileDirectory)Microsoft.Extensions.FileSystemGlobbing\**\*csproj" />
<PortedExtensionsProject Include="$(MSBuildThisFileDirectory)Microsoft.Extensions.Hosting.Abstractions\**\*csproj" />
<PortedExtensionsProject Include="$(MSBuildThisFileDirectory)Microsoft.Extensions.Logging.Abstractions\**\*csproj" />
<PortedExtensionsProject Include="$(MSBuildThisFileDirectory)Microsoft.Extensions.Options.*\**\*csproj" />
<PortedExtensionsProject Include="$(MSBuildThisFileDirectory)Microsoft.Extensions.Primitives\**\*csproj" />
<ProjectExclusions Include="$(MSBuildThisFileDirectory)*\ref\**\Microsoft.Extensions.*proj" Exclude="@(PortedExtensionsProject)" />
<ProjectExclusions Include="$(MSBuildThisFileDirectory)*\src\**\Microsoft.Extensions.*proj" Exclude="@(PortedExtensionsProject)" />
<ProjectExclusions Include="$(MSBuildThisFileDirectory)*\tests\Microsoft.Extensions.*.Tests.csproj" Exclude="@(PortedExtensionsProject)" />
<ProjectExclusions Include="$(MSBuildThisFileDirectory)*\tests\FunctionalTests\Microsoft.Extensions.Configuration.Functional.Tests.csproj" />
<ProjectExclusions Include="$(MSBuildThisFileDirectory)*\tests\Common\test\Microsoft.Extensions.Logging.Testing.Tests.csproj" />
<ProjectExclusions Include="$(MSBuildThisFileDirectory)*\tests\FunctionalTests\Microsoft.Extensions.Hosting.Functional.Tests.csproj" />
<ProjectExclusions Include="$(MSBuildThisFileDirectory)*\tests\TestApp\Microsoft.Extensions.Hosting.TestApp.csproj" />
<ProjectExclusions Include="$(MSBuildThisFileDirectory)*\tests\UnitTests\Microsoft.Extensions.Hosting.Unit.Tests.csproj" />
<ProjectExclusions Include="$(MSBuildThisFileDirectory)*\tests\Common\Microsoft.Extensions.Logging.Tests.csproj" />
<ProjectExclusions Include="$(MSBuildThisFileDirectory)Common\tests\Extensions\**\*.csproj" />
</ItemGroup>
. /cc @maryamariyan

@Anipik
Copy link
Contributor Author

Anipik commented Mar 19, 2020

We are missing the DependencyInjection Project in the list

@ericstj
Copy link
Member

ericstj commented Mar 19, 2020

Or maybe pkgproj exists and package.builds isn't honoring exclusion. @maryamariyan is DI ready to build (ref/src/test/pkg)?

@maryamariyan
Copy link
Member

maryamariyan commented Mar 19, 2020

NO. DI build is OK but not tests. Tests is why I disabled it

@maryamariyan
Copy link
Member

here: #33816

@Anipik
Copy link
Contributor Author

Anipik commented Mar 19, 2020

NO. DI build is OK but not tests. Tests is why I disabled it

we can build src and ref and skip the tests in order to unblock here. @maryamariyan what is causing the dependency Injection src project to build ? is it the package builds ?

@ericstj
Copy link
Member

ericstj commented Mar 20, 2020

what is causing the dependency Injection src project to build ? is it the package builds ?

Yeah, the existence of pkgproj will cause that. We didn't exclude pkgproj from the *.builds file. If you wanted to make progress on this you could rename the pkgproj then let @maryamariyan rename it back, or just wait for her fix. I'd prefer the latter since this isn't blocking anything.

@Anipik Anipik merged commit 535d957 into dotnet:master Mar 24, 2020
@Anipik Anipik deleted the fixClean branch March 24, 2020 14:18
ViktorHofer added a commit to ViktorHofer/runtime that referenced this pull request Jul 23, 2020
ViktorHofer added a commit that referenced this pull request Jul 23, 2020
- Remove depprojs which currently binplace external references into the RefPath folders in favor of PackageReference and PackageDownload items.
- Build all configurations by default when building an individual project (either on the CLI or inside VS) same as with the official SDK. This enables .NETFramework Test Explorer support.
- Centrally define libraries that compose the shared framework instead of in each Directory.Build.props file to be able to build the targeting pack first and consume it in the OOB libraries.
- Use ProjectReferences to reference OOB projects. Compile against the reference assembly but use the implementation assembly app-local during runtime.
- Remove OOBs from the testhost and remove the testhost folder for .NETFramework as it isn't required anymore.
- Only binplace for $(NetCoreAppCurrent) to compose a) the targeting pack, b) the runtime pack, c) the testhost, d) a full closure for the shims.
- Use Targeting Packs for OOB projects (with their implicit assembly references) but still explicitly define granular references for .NETCoreApp configurations (DisableImplicitAssemblyReferences switch). Use the implicit targeting pack references in some Microsoft.Extensions.* cases.
- Remove placeholder configurations as they aren't needed anymore with explicit P2Ps vs Targeting Pack references.
- Remove implicit assembly references (ie for .NETFramework, mscorlib)
- Remove AssemblySearchPath hacks that were introduced with b7c4cb7 as the targeting pack is now used by default.
- Reduce unnecessary .NETFramework configurations that were added to run tests in favor of the already existing ref&src configurations.
- Stop hardcoding the paths for wasm assemblies and use the returned TargetPath of the ProjectReferences.
- Addressed formatting (ItemGroups, References at the bottom of the project file, ordering of references, use LibrariesProjectRoot instead of a relative path, unnecessary AssemblyName and RootNamespace properties which are identical to the project name, ordering of tfms)
- Revert "fix clean (#33758)"
Jacksondr5 pushed a commit to Jacksondr5/runtime that referenced this pull request Aug 10, 2020
- Remove depprojs which currently binplace external references into the RefPath folders in favor of PackageReference and PackageDownload items.
- Build all configurations by default when building an individual project (either on the CLI or inside VS) same as with the official SDK. This enables .NETFramework Test Explorer support.
- Centrally define libraries that compose the shared framework instead of in each Directory.Build.props file to be able to build the targeting pack first and consume it in the OOB libraries.
- Use ProjectReferences to reference OOB projects. Compile against the reference assembly but use the implementation assembly app-local during runtime.
- Remove OOBs from the testhost and remove the testhost folder for .NETFramework as it isn't required anymore.
- Only binplace for $(NetCoreAppCurrent) to compose a) the targeting pack, b) the runtime pack, c) the testhost, d) a full closure for the shims.
- Use Targeting Packs for OOB projects (with their implicit assembly references) but still explicitly define granular references for .NETCoreApp configurations (DisableImplicitAssemblyReferences switch). Use the implicit targeting pack references in some Microsoft.Extensions.* cases.
- Remove placeholder configurations as they aren't needed anymore with explicit P2Ps vs Targeting Pack references.
- Remove implicit assembly references (ie for .NETFramework, mscorlib)
- Remove AssemblySearchPath hacks that were introduced with b7c4cb7 as the targeting pack is now used by default.
- Reduce unnecessary .NETFramework configurations that were added to run tests in favor of the already existing ref&src configurations.
- Stop hardcoding the paths for wasm assemblies and use the returned TargetPath of the ProjectReferences.
- Addressed formatting (ItemGroups, References at the bottom of the project file, ordering of references, use LibrariesProjectRoot instead of a relative path, unnecessary AssemblyName and RootNamespace properties which are identical to the project name, ordering of tfms)
- Revert "fix clean (dotnet#33758)"
@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2020
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.

Unable to build System.Runtime.WindowsRuntime
5 participants