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

Add conditional to not build net46 in sourcebuild #2309

Closed

Conversation

dseefeld
Copy link
Contributor

@dseefeld dseefeld commented Jun 5, 2018

A recent commit b16a488 to enable x-plat net46 build causes the source-build build to fail for 2.1.

@eerhardt
Copy link
Member

eerhardt commented Jun 5, 2018

/cc @nguerrera - who changed this recently: dseefeld@b16a488#diff-83d1b9d720377aaf70c666e9e75befd6

@eerhardt eerhardt requested a review from nguerrera June 5, 2018 16:08
@nguerrera
Copy link
Contributor

@weshaggard assured me this would work. The whole point of that commit was to get these binaries into source build.

Copy link
Contributor

@nguerrera nguerrera left a comment

Choose a reason for hiding this comment

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

Waiting to understand why we can't use the reference assembly package in source build as promised.

@nguerrera
Copy link
Contributor

See dotnet/source-build#125 (comment)

We are using the same reference assembly packages as roslyn tools.

@eerhardt
Copy link
Member

eerhardt commented Jun 5, 2018

FYI - @weshaggard is OOF.

@nguerrera - can you help me understand where Microsoft.NETFramework.ReferenceAssemblies is hosted? I can't find it on nuget.org or on https://dotnet.myget.org/gallery/dotnet-core. Maybe to fix source-build we just need to add a new feed URL?

@crummel
Copy link
Contributor

crummel commented Jun 5, 2018

I think we'll also need to change MSBuild:
https://github.com/Microsoft/msbuild/blob/ac19036b0de4f1b58c3be76d51000df8db98560c/src/Directory.Build.props#L27
once we get the reference assemblies sorted out since SDK is referencing the MSBuild assemblies we produce.

@nguerrera
Copy link
Contributor

AFAICT, we get Microsoft.NETFramework.ReferenceAssemblies from https://dotnet.myget.org/F/roslyn-tools/api/v3/index.json

@nguerrera
Copy link
Contributor

It was my understanding from dotnet/source-build#125 that others in source-build were already using this.

@crummel
Copy link
Contributor

crummel commented Jun 5, 2018

I don't think anyone is now but I've got MSBuild building on its own with these changes. I'm going to try something similar with SDK now that that reference is unblocked if that looks reasonable.

@nguerrera
Copy link
Contributor

@crummel, hmm that's a different package than the one from roslyn-tools. Wes specifically mentioned roslyn tools so I'm confused. But if we can get this building in source build with that, ok.

@nguerrera
Copy link
Contributor

I think it would be good if source build could use Microsoft.NETFramework.ReferenceAssemblies because that one is based on the proposal that we intend to ship and it handles setting things up for you (no need to muck with FrameworkPathOverride, etc.)

@weshaggard
Copy link
Member

Yes lets try to use Microsoft.NETFramework.ReferenceAssemblies and move it to the dotnet-core feed if we need to. I was pretty sure it was already being used as a pre-built but if it isn't then we can and should start doing so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants