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

Disable producing targeting packs in servicing builds (unless patching) #3827

Closed
dagood opened this issue Nov 1, 2019 · 7 comments
Closed
Assignees
Milestone

Comments

@dagood
Copy link
Member

dagood commented Nov 1, 2019

https://github.com/dotnet/core-setup/issues/8507 tracks disabling the NETStandard targeting pack for 3.1 in particular. However, we should disable NETCoreApp and WindowsDesktop's as well to match this part of the framework pack design:

Targeting pack package versions will generally not increase past major.minor.0 where major.minor matches the the corresponding two-part TFM. For example, when there is a 3.0.1 .NET Core runtime, the targeting pack will likely remain at 3.0.0. This follows from the fact that targeting packs represent public API surface, which must not change in a patch version of the runtime. (With that said, we can reserve the right to modify the targeting pack in a patch release to fix a severe bug. In the rare event that this occurs, the targeting pack patch version could be incremented past 0.)

In theory, there isn't a problem with Core-Setup building the targeting packs as long as they aren't published and downstream repos don't point to the bad new version. However, this requires manual dependency PR fixup/pinning by the downstream repos. It would be friendlier for Core-Setup to not produce the bad versions in the first place. This also means that to turn on the build, we only need to change Core-Setup, rather than undo dependency pins in any downstream repos that have them.

/cc @nguerrera @mmitche @wtgodbe

@dagood
Copy link
Member Author

dagood commented Nov 4, 2019

This is tied up with a general source-build problem... we need the old targeting pack to assemble the current SDK installers, so how will the old targeting pack source-built?

E.g. the 3.0.108 SDK installer could need the 3.0.0 targeting pack and 3.0.7 runtime.

This is the same core problem as dotnet/source-build#923. I'm asking offline what the source-build team's current thoughts are on this.

My current understanding is that source-build generally assumes that the current sources are close enough to end up producing a working build when put together. In this case, I think that would mean forcing the targeting pack to always build when doing a source-build, and forcing the targeting pack's version back to the last-shipped version so that it can target the same runtimes as the Microsoft build. (Keeping source-build parity.)

/cc @MichaelSimons @leecow @dseefeld @crummel @adaggarwal

@dagood dagood self-assigned this Nov 8, 2019
@jzabroski
Copy link
Contributor

jzabroski commented Dec 2, 2019

@dagood You're pretty close to the heart of the problem, but it actually goes deeper and simpler than that:

Imagine I install a .NET Global Tool called TaskRunner which dynamically loads assemblies containing Tasks, which in turn dynamically link to a version of the eponimous System.ComponentModel.Annotations. There is a three-way dependency between the tool, the tasks, and the tasks and the target runtime the tasks were built against. Further assume a transitive dependency between your task and System.ComponentModel.Annotations, called TaskCommon.dll.

You Cannot solve this problem with brute force.

I've actually taken a stab at trying to fix all this, but the problem is everything is written in MSBuild XML and reasoning about a language with a global variable namespace is very unproductive.

The reason I chose System.ComponentModel.Annotations is that, for the love of unnecessity, this library is re-released every runtime release. This is your source-build problem, at its core.

@dagood
Copy link
Member Author

dagood commented Dec 2, 2019

@jzabroski I don't follow what problem you're trying to illustrate, maybe it would help to explain the problem you're seeing more directly, if possible?


I've actually taken a stab at trying to fix all this, but the problem is everything is written in MSBuild

I highly recommend http://msbuildlog.com/, it's pretty much necessary to get along with MSBuild IMO.

@jzabroski
Copy link
Contributor

I'll try to repro it in the coming weeks. It's a meta problem - there are so many abstraction boundaries above the "this is the path to my dll" abstraction boundary but my point is none of that provenance information matters because the abstraction boundaries don't seem to work. I have not had the time to fully articulate it, in part because I was waiting for Nick to complete his RFC - hard to really judge something without a spec. dotnet/designs#50 (comment)

I am aware of MSbuildlog. I was not aware of the new binlog feature added since 15.3.

@jzabroski
Copy link
Contributor

There is one, and only one, scenario where the current abstraction boundaries work: quasi-static linking. This is what the new SDK tools do by generating a .deps.json file. If you need interstitial glue, there is no solution that works. Maybe I misunderstand what this issue and all the linked issues are about, but to me its interstitial glue code to make runtime and targeting packs "stick together". That basic mechanism is broken to me if you have to do so much effort. There needs to be a new glue layer called Epoxy that just forms a cohesive bond right where you need it. Joking, metaphorically, but seriously.

@dagood
Copy link
Member Author

dagood commented Dec 2, 2019

That sounds like a completely different problem, but still pretty vague... this issue has pretty narrow scope to our repository build infrastructure needing to be a bit quirky to deal with the product repos' dependency flow and release processes. It's only open until I port dotnet/core-setup#8824 to dotnet/runtime.

@dagood
Copy link
Member Author

dagood commented Jan 14, 2020

Closing: the current impl has a known issue, and I'll use that issue to also track applying the fixed version of this code to the dotnet/runtime master branch: #639

@dagood dagood closed this as completed Jan 14, 2020
@msftgits msftgits transferred this issue from dotnet/core-setup Jan 30, 2020
@msftgits msftgits added this to the 3.1 milestone Jan 30, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants