Skip to content

Conversation

dagood
Copy link
Member

@dagood dagood commented Oct 15, 2021

This prebuilt package filled a gap in the official 1.0 version of the .NET Framework reference assembly packages. The gap has been fixed in later versions of the official packages. Removal fixes a prebuilt dependency by letting the repo use the official version of the packages, which are produced by SBRP.

I removed <BypassFrameworkInstallChecks>true</BypassFrameworkInstallChecks> because the comment essentially says that it shouldn't be necessary anymore and it sounds like a good idea to enable checks when I can, but I'm not actually familiar with what the checks do. 😄

Fixes #6935

This prebuilt package filled a gap in the official 1.0 version of the
.NET Framework reference assembly packages. The gap has been fixed in
later versions of the official packages. Removal fixes a prebuilt
dependency by letting the repo use the official version of the packages,
which are produced by SBRP.
@dagood dagood force-pushed the use-official-net35 branch from 6aff19b to 52573bb Compare October 15, 2021 19:25
@rainersigwald
Copy link
Member

Do you want this for 17.0/6.0GA, or just "the next release" 17.1/6.0.2xx?

@dagood
Copy link
Member Author

dagood commented Oct 15, 2021

Ah, yeah, 6.0 GA, so we can remove the patch in dotnet/installer for servicing releases (and get more accurate prebuilt detection in this repo itself). Same for #6967. Sounds like the vs17.0 branch. 😄

What porting strategy does this repo use--cherry-pick to release branch after/simultaneously with main, or just PR to feature branch which eventually gets merged back into main?

@rainersigwald
Copy link
Member

PR to release branch which gets merged back to main in MSBuild, please.

I have no objection to these but we may have to have a discussion on our side about getting them in, since we're in QB mode for VS. I suspect we can roll these plus #6902 together as "required infra changes for GA"--@marcpopMSFT, do you have a preference?

@rainersigwald rainersigwald added this to the MSBuild 17.0 milestone Oct 15, 2021
@dagood
Copy link
Member Author

dagood commented Oct 15, 2021

I'm checking with the source-build team on the priority. I'm just temporarily helping out with source-build right now and I don't have full context on these patch application PRs. It doesn't seem necessary to me to get it through QB (we'll just have a patch in dotnet/installer for 6.0.0 that hopefully can make it into servicing for 6.0.1) but I don't want to say that definitively.

@dagood
Copy link
Member Author

dagood commented Oct 15, 2021

This is ok to defer until servicing. @MichaelSimons confirmed that we hit QB mode with vstest, too, and also deferred there.

@marcpopMSFT
Copy link
Member

Sounds like 17.1 then

@dagood
Copy link
Member Author

dagood commented Oct 18, 2021

PR to release branch which gets merged back to main in MSBuild, please.

I just noticed I'd misread this as "when it gets merged" and I was waiting for that. 😅 I'll rebase the PRs on vs17.1.

@rainersigwald
Copy link
Member

I'll rebase the PRs on vs17.1.

No need! main will flow into vs17.1 when we fork for 17.2.

@ladipro ladipro merged commit 808b2ae into dotnet:main Oct 22, 2021
@dagood dagood deleted the use-official-net35 branch October 22, 2021 16:43
@KirillOsenkov
Copy link
Member

Hmm @dagood I'm building on a machine without .NET 3.5 SP1 ref assemblies installed and I'm getting this?

image

Have we tested building MSBuild with this on a clean machine?

@KirillOsenkov
Copy link
Member

Hmm, still 1.0.0 here:

<GlobalPackageReference Include="Microsoft.NETFramework.ReferenceAssemblies" Version="1.0.0" PrivateAssets="All"/>

@dagood
Copy link
Member Author

dagood commented Oct 22, 2021

Have we tested building MSBuild with this on a clean machine?

Not on a Windows machine, no. It works in the context of a clean source-build.

@dagood
Copy link
Member Author

dagood commented Oct 22, 2021

Hmm, still 1.0.0 here:

<GlobalPackageReference Include="Microsoft.NETFramework.ReferenceAssemblies" Version="1.0.0" PrivateAssets="All"/>

Ah, yep, I think that would downgrade from whatever the SDK's trying to add automatically (probably 1.0.2) down to 1.0.0. I think that itemgroup just needs to be removed, like it is for source-build in the condition:

<ItemGroup Condition="'$(DotNetBuildFromSource)' != 'true'">
<GlobalPackageReference Include="Microsoft.NETFramework.ReferenceAssemblies" Version="1.0.0" PrivateAssets="All"/>
</ItemGroup>

I think it would show up in a non-source-build build on Linux, too. I can try a quick removal.

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.

Remove prebuilt dependency on jnm2.ReferenceAssemblies.net35, now handled by Microsoft.NETFramework.ReferenceAssemblies

5 participants