Skip to content

Conversation

@mthalman
Copy link
Member

Contributes to dotnet/source-build#3043

Patch changes backported at dotnet/arcade#13009

@mthalman
Copy link
Member Author

Oh, interesting. This apparently is causing an issue later on in the build in symreader that I need to investigate. Changing this to draft for now.

@mthalman mthalman marked this pull request as draft April 13, 2023 20:35
Comment on lines +34 to +46
diff --git a/src/Microsoft.DotNet.GenFacades/Microsoft.DotNet.GenFacades.csproj b/src/Microsoft.DotNet.GenFacades/Microsoft.DotNet.GenFacades.csproj
index d73f6abd..20c0c33b 100644
--- a/src/Microsoft.DotNet.GenFacades/Microsoft.DotNet.GenFacades.csproj
+++ b/src/Microsoft.DotNet.GenFacades/Microsoft.DotNet.GenFacades.csproj
@@ -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>
-
Copy link
Member

Choose a reason for hiding this comment

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

This patch is no longer necessary. Remove.

Comment on lines +13 to +30
diff --git a/src/Microsoft.DotNet.CodeAnalysis/Microsoft.DotNet.CodeAnalysis.csproj b/src/Microsoft.DotNet.CodeAnalysis/Microsoft.DotNet.CodeAnalysis.csproj
index d1ac5ccb..5cdfd539 100644
--- a/src/Microsoft.DotNet.CodeAnalysis/Microsoft.DotNet.CodeAnalysis.csproj
+++ b/src/Microsoft.DotNet.CodeAnalysis/Microsoft.DotNet.CodeAnalysis.csproj
@@ -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>
-
Copy link
Member

Choose a reason for hiding this comment

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

This patch is no longer necessary either.

Comment on lines +20 to +23
+ <Dependency Name="System.IO.Packaging" Version="4.5.0">
+ <Uri>https://github.com/dotnet/corefx</Uri>
+ <Sha>30ab651fcb4354552bd4891619a0bdd81e0ebdbf</Sha>
+ </Dependency>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
+ <Dependency Name="System.IO.Packaging" Version="4.5.0">
+ <Uri>https://github.com/dotnet/corefx</Uri>
+ <Sha>30ab651fcb4354552bd4891619a0bdd81e0ebdbf</Sha>
+ </Dependency>
+ <Dependency Name="System.IO.Packaging" Version="7.0.0">
+ <Uri>https://github.com/dotnet/runtime</Uri>
+ <Sha>30ab651fcb4354552bd4891619a0bdd81e0ebdbf</Sha>
+ </Dependency>

Comment on lines +26 to +29
+ <Dependency Name="Microsoft.Extensions.DependencyModel" Version="6.0.0">
+ <Uri>https://github.com/dotnet/runtime</Uri>
+ <Sha>8470979eb44c2218025515234d3e01138bd74afb</Sha>
</Dependency>
Copy link
Member

Choose a reason for hiding this comment

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

This is outdated as well but I'm not sure what the version should be.

@MichaelSimons
Copy link
Member

Closing as obsolete. See #16592.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants