Use ProjectReferences in inbox src projects #106329
Open
+889
−905
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Aug 13, 2024
ViktorHofer
force-pushed
the
UseP2PsEverywhere
branch
from
38dad8e
to
9713954
Compare
ViktorHofer
changed the title
ViktorHofer
force-pushed
the
UseP2PsEverywhere
branch
6 times, most recently
from
dc972d0
to
3fdee85
Compare
This was referenced Aug 16, 2024
ViktorHofer
force-pushed
the
UseP2PsEverywhere
branch
2 times, most recently
from
d23e741
to
8a94707
Compare
ViktorHofer
changed the title
|
Open question: Should PrivateAssets="all" be the default for P2Ps pointing to sfx libraries? IMO that would make things easier. EDIT: Yes we should. Did so in the second commit. |
This was referenced Aug 17, 2024
Closed
ViktorHofer
force-pushed
the
UseP2PsEverywhere
branch
2 times, most recently
from
fad9f15
to
e5e52a0
Compare
ViktorHofer
force-pushed
the
UseP2PsEverywhere
branch
from
079724c
to
4cf64df
Compare
|
Blocked on the NuGet tooling fix: NuGet/Home#10368 |
|
Damn, the NuGet tooling bug still affects this and makes the PR impossible to merge. As an example, see https://gist.github.com/ViktorHofer/d7c4c5dfca25ea5936ae878808ec1a93 Search for "type": "project". It lists a lot of P2Ps for net9.0 that shouldn't exist. Those dependencies should come in via package references (coming transitively in from the CodeAnalysis package refs). When I comment out the .NET Framework P2P to System.Text.Json I get the correct project.assets.json content for net9.0 (again search for type project): https://gist.github.com/ViktorHofer/9aef4bd9382519eecd990537d63eb0a3 |
Open
3 tasks
ViktorHofer
added
the
NO-MERGE
There are numerous benefits in using ProjectReferences consistently in all libraries: 1. An upfront "libs" build isn't required anymore and sfx libraries can now directly be built from a fresh clone (with dotnet build or inside VS). I.e. `dotnet.cmd pack src/libraries/System.Text.Json/src/` is now possible from a fresh clone. 2. Because of 1), we can now add a solution file for the whole sfx that can directly be opened and worked with from a fresh clone. 3. The overall root build is faster. Without this change, the build order was sfx-ref -> (sfx-src & sfx-gen) so the shared framework reference projects first had to be built and only then the sfx src and gen projects could be built. Now with this change, everything gets built in parallel. 4. Using P2Ps means that we now follow the common and well supported msbuild and SDK path instead of repo customization. The downside of doing this is that the dependency graph gets bigger, meaning that more projects get incrementally built when doing a "dotnet build". This is nothing new and the SDK team recommends to pass the "--no-dependencies" flag to "dotnet build" if incrementally (no-op) building the additional dependency nodes is noticeable. This is less of a concern inside VS as that has a "fast up-to-date check" feature that doesn't even attempt to build projects that didn't change. For VS, really the only noticeable change is that the solution explorer now lists more projects and that when opening a solution, more projects need to be evaluated. But, that should be fast enough when using an up-to-date version of VS. - A few observations that make the change more involved: There's a NuGet client bug that requires a few workarounds: NuGet/Home#10368 Because of that, as a workaround, PackageId had to be set to a different string for S.Numerics.Vectors and System.Memory. We should fix the NuGet tooling issue to eventually get rid of the workarounds introduced with this commit. There was already a PR in NuGet.Client open but it was closed because of staleness. - System.Data.Common.csproj is a weird project as it references CoreLib and reference assemblies. I had to disable transitive project references in order for type universes to not clash and explicitly set CompileUsingReferenceAssemblies=true as that gets set to false when the library explicitly references CoreLib.
ViktorHofer
force-pushed
the
UseP2PsEverywhere
branch
from
4cf64df
to
1725657
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #93877
There are numerous benefits in using ProjectReferences consistently
in all libraries:
can now directly be built from a fresh clone (with dotnet build or inside
VS). I.e.
dotnet.cmd pack src/libraries/System.Text.Json/src/isnow possible from a fresh clone.
can directly be opened and worked with from a fresh clone.
order was sfx-ref -> (sfx-src & sfx-gen) so the shared framework
reference projects first had to be built and only then the sfx src
and gen projects could be built. Now with this change, everything
gets built in parallel.
msbuild and SDK path instead of repo customization.
The downside of doing this is that the dependency graph gets bigger,
meaning that more projects get incrementally built when doing a "dotnet
build". This is nothing new and the SDK team recommends to pass the
"--no-dependencies" flag to "dotnet build" if incrementally (no-op)
building the additional dependency nodes is noticeable.
This is less of a concern inside VS as that has a "fast up-to-date check"
feature that doesn't even attempt to build projects that didn't change.
For VS, really the only noticeable change is that the solution explorer
now lists more projects and that when opening a solution, more projects
need to be evaluated. But, that should be fast enough when using an
up-to-date version of VS.