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

Updates to enable PVP flow #13009

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions eng/Version.Details.xml
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,12 @@
<Uri>https://github.com/dotnet/deployment-tools</Uri>
<Sha>b60c95e1ce736630d17e16626c59e3dd85ebae2b</Sha>
</Dependency>
<!-- Necessary for source-build. This allows the package to be retrieved from previously-source-built artifacts
and flow in as dependencies of the packages produced by arcade. -->
<Dependency Name="Microsoft.Extensions.DependencyModel" Version="6.0.0">
Copy link
Member

Choose a reason for hiding this comment

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

IMO, both of these new dependencies should be live versions otherwise there is a significant disconnect between the repo level source-build and the production source-build.

Copy link
Member Author

Choose a reason for hiding this comment

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

How can they be live dependencies when arcade gets built first? Or am I misunderstanding what is meant by live dependencies?

Copy link
Member

Choose a reason for hiding this comment

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

By live dependency I mean there is a darc subscription to update it on a specific cadence to latest. In source-build it will pick up the version from previously-source-built.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think that's the right approach unless we think that the surface area is changing in such a way that it is likely to break when built against latest. This repo isn't taking advantage of new surface area.

Copy link
Member

Choose a reason for hiding this comment

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

So I can better understand your viewpoint @mmitche, does .NET 9 change your viewpoint at all?

Copy link
Member

Choose a reason for hiding this comment

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

Not at first glance. Let me think on it.

Copy link
Member

Choose a reason for hiding this comment

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

I was just talking with @oleksandr-didyk and remembered why 6.0.0 (an SBRP) won't work for the repo source-build scenario. The Arcade components that reference Microsoft.Extensions.DependencyModel are loaded/used in downstream repos. When an SBRP is referenced an exception will be thrown. @oleksandr-didyk discovered this earlier in the runtime source-build leg. @mthalman discovered this in the product source-build which is why he is declaring this verion.details dependency is needed.

<Uri>https://github.com/dotnet/runtime</Uri>
<Sha>8470979eb44c2218025515234d3e01138bd74afb</Sha>
</Dependency>
<!-- Explicit dependency because Microsoft.Deployment.DotNet.Releases has different versioning
than the SB intermediate -->
<Dependency Name="Microsoft.SourceBuild.Intermediate.deployment-tools" Version="8.0.0-preview.1.23159.4" CoherentParentDependency="Microsoft.NET.Sdk.WorkloadManifestReader">
Expand All @@ -107,5 +113,11 @@
<Sha>39aef81ec6cffa06da9964b46d4b9e3bf2fc9979</Sha>
<SourceBuild RepoName="sdk" ManagedOnly="true" />
</Dependency>
<!-- Necessary for source-build. This allows the package to be retrieved from previously-source-built artifacts
and flow in as dependencies of the packages produced by arcade. -->
<Dependency Name="System.IO.Packaging" Version="4.5.0">
<Uri>https://github.com/dotnet/corefx</Uri>
<Sha>30ab651fcb4354552bd4891619a0bdd81e0ebdbf</Sha>
</Dependency>
</ToolsetDependencies>
</Dependencies>
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,6 @@
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="$(MicrosoftCodeAnalysisCSharpVersion)" ExcludeAssets="analyzers" />
</ItemGroup>

<!--
In the source-build tarball build, Microsoft.CodeAnalysis.CSharp has dependencies on old
versions of these packages due to repo build order. Override to lift them to the versions passed
in via DotNetPackageVersionPropsPath.
-->
<ItemGroup Condition="'$(DotNetBuildFromSource)' == 'true'">
<PackageReference Include="System.Collections.Immutable" Version="$(SystemCollectionsImmutableVersion)" />
<PackageReference Include="System.Reflection.Metadata" Version="$(SystemReflectionMetadataVersion)" />
</ItemGroup>

<ItemGroup>
<Content Include="build/*.*" PackagePath="build" />
<Content Include="content/*.*" PackagePath="content" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,6 @@
<PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="$(MsbuildTaskMicrosoftCodeAnalysisCSharpVersion)" ExcludeAssets="analyzers" Publish="false" />
</ItemGroup>

<!-- When building offline we need to bump the version of System.Reflection.Metadata that CodeAnalysis package depends on to match what the source build tarball expects. -->
<ItemGroup Condition="'$(DotNetBuildOffline)' == 'true'">
<PackageReference Include="System.Reflection.Metadata" Version="$(SystemReflectionMetadataVersion)" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFramework)' == 'net472'">
<Compile Include="..\Common\Internal\AssemblyResolver.cs" />
<Compile Include="..\Common\Internal\BuildTask.Desktop.cs" />
Expand Down