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

Enable source-build pre-built detection #81468

Closed
oleksandr-didyk opened this issue Feb 1, 2023 · 24 comments · Fixed by #86151
Closed

Enable source-build pre-built detection #81468

oleksandr-didyk opened this issue Feb 1, 2023 · 24 comments · Fixed by #86151
Assignees

Comments

@oleksandr-didyk
Copy link
Contributor

Part of dotnet/source-build#3017

Enable source-build pre-build detection on the current repository and resolve any pre-build issues discovered

@oleksandr-didyk oleksandr-didyk self-assigned this Feb 1, 2023
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Feb 1, 2023
@oleksandr-didyk
Copy link
Contributor Author

As part of work on the task we discovered that in the context of repo build runtime's source-build is utilizing older versions of several components, specifically:

In the context of current product source-build, these dependencies will be replaced by their respective latest versions. This creates a sizable difference in behaviour between the product source-build and the repo source-build.

@ViktorHofer pinging you here to get an opinion on what, in your mind, is a better option from runtime's perspective -> to have source-build continue using the latest versions (i.e. source-build differs from 'VS build') or for source-build to use the VS-compatible ones (i.e. source-build behaves the same as 'VS build')? The later will become possible with the introduction of repository ProjectVersions.props files.

CC: @MichaelSimons @mmitche

@ghost
Copy link

ghost commented Feb 1, 2023

Tagging subscribers to this area: @dotnet/runtime-infrastructure
See info in area-owners.md if you want to be subscribed.

Issue Details

Part of dotnet/source-build#3017

Enable source-build pre-build detection on the current repository and resolve any pre-build issues discovered

Author: oleksandr-didyk
Assignees: oleksandr-didyk
Labels:

area-Infrastructure, untriaged

Milestone: -

@mmitche
Copy link
Member

mmitche commented Feb 1, 2023

As part of work on the task we discovered that in the context of repo build runtime's source-build is utilizing older versions of several components, specifically:

In the context of current product source-build, these dependencies will be replaced by their respective latest versions. This creates a sizable difference in behaviour between the product source-build and the repo source-build.

@ViktorHofer pinging you here to get an opinion on what, in your mind, is a better option from runtime's perspective -> to have source-build continue using the latest versions (i.e. source-build differs from 'VS build') or for source-build to use the VS-compatible ones (i.e. source-build behaves the same as 'VS build')? The later will become possible with the introduction of repository ProjectVersions.props files.

CC: @MichaelSimons @mmitche

This changed last week. Runtime now builds very early in the stack, mirroring the way the product builds. Therefore roslyn and msbuild do not flow into runtime.

I think the cases above are textbook examples of SBRPs.

@oleksandr-didyk
Copy link
Contributor Author

oleksandr-didyk commented Feb 7, 2023

Pre-builts currently getting removed / excluded:

Pre-builts that ideally would require a version bump. @ViktorHofer would be ideal if we could know which of these can be updated (we would create separate issues for the update + allow the pre-builts in the infra until the said issue would get attention from runtime) and which for whatever reason cannot:

  • Microsoft.Build 17.3.2 -> while creating SBRPs for the package and its dependencies would not be a hard process, this might create additional maintenance for the runtime team once the version will need a bump. From Git Blame, it seems that the last update to the package was a while back and was not related to any specific task - Update a few dependencies #77678
  • Nuget.ProjectModel 6.2.2
  • Microsoft.Extensions.DependencyModel 6.0.0
  • System.CommandLine 2.0.0-beta4.22355.1 -> from trying out the latest version, it seems that for this package to be bumped source changes are required, as the package's API changed

@ViktorHofer
Copy link
Member

ViktorHofer commented Feb 7, 2023

Microsoft.NETFramework.ReferenceAssemblies -> would be removed once TFM filtering is enabled dotnet/arcade#12310

We already have our own infrastructure to exclude these if that's what we want. Do we have a tracking issue for that? I wasn't aware that we want to remove .NET Framework pre-builts right now. What's that issue's priority?

Microsoft.Build 17.3.2
Nuget.ProjectModel 6.2.2

AFAIK we are free to use any version here, i.e. the latest version from nuget.org. cc @dotnet/runtime-infrastructure
In the long-term, how do we keep these up-to-date and in sync with the rest of the VMR?

Microsoft.Extensions.DependencyModel 6.0.0

We build that library ourselves in the repository so we should remove the prebuilt package dependency entirely.

System.CommandLine 2.0.0-beta4.22355.1 -> from trying out the latest version, it seems that for this package to be bumped source changes are required, as the package's API changed

cc @adamsitnik

@oleksandr-didyk
Copy link
Contributor Author

Microsoft.NETFramework.ReferenceAssemblies -> would be removed once TFM filtering is enabled dotnet/arcade#12310

We already have our own infrastructure to exclude these if that's what we want. Do we have a tracking issue for that? I wasn't aware that we want to remove .NET Framework pre-builts right now. What's that issue's priority?

We don't have a tracking issue just yet, since it would depend on when the changes would be usable in runtime. IIRC we would allow them as pre-builts if need be and remove them once the filtering is available and enabled.

Microsoft.Build 17.3.2
Nuget.ProjectModel 6.2.2

AFAIK we are free to use any version here, i.e. the latest version from nuget.org. cc @dotnet/runtime-infrastructure In the long-term, how do we keep these up-to-date and in sync with the rest of the VMR?

If we do bump them to latest - Maestro dependency flow would update them automatically trough subscriptions that would be created as part of this task. Other repositories would get the packages from Maestro as well, so it should sync up

Microsoft.Extensions.DependencyModel 6.0.0

We build that library ourselves in the repository so we should remove the prebuilt package dependency entirely.

Sounds great, I will create a tracking issue for it + add a comment to remove the allowed pre-built as part of that work

@ViktorHofer
Copy link
Member

We don't have a tracking issue just yet, since it would depend on when the changes would be usable in runtime. IIRC we would allow them as pre-builts if need be and remove them once the filtering is available and enabled.

As said, TFM filtering IS already available in dotnet/runtime. Nothing blocks us from removing .NET Framework nodes from the source-build graph. If this is desirable and important, please file an issue for that (including the priority for the source build team).

through subscriptions that would be created as part of this task

Sounds like a SRBP source build subscription. Is that right?

Sounds great, I will create a tracking issue for it + add a comment to remove the allowed pre-built as part of that work

Thanks 👍

@oleksandr-didyk
Copy link
Contributor Author

We don't have a tracking issue just yet, since it would depend on when the changes would be usable in runtime. IIRC we would allow them as pre-builts if need be and remove them once the filtering is available and enabled.

As said, TFM filtering IS already available in dotnet/runtime. Nothing blocks us from removing .NET Framework nodes from the source-build graph. If this is desirable and important, please file an issue for that (including the priority for the source build team).

My bad, I worded it badly. I was just providing context for why the issue wasn't raised. I will create and issue for it once I double-check its priority. Thanks for pointing this out

through subscriptions that would be created as part of this task

Sounds like a SRBP source build subscription. Is that right?

We would create subscriptions for the packages themselves, so for msbuild and nuget repos. The repos source-build their latest versions as part of their CI, which is then available for runtime's source-build restore from the *-transport NuGet feeds.

@ViktorHofer
Copy link
Member

ViktorHofer commented Feb 7, 2023

We would create subscriptions for the packages themselves, so for msbuild and nuget repos.

I thought we can't do that as runtime now builds before msbuild and nuget in the source-build graph? Above from @mmitche:

This changed last week. Runtime now builds very early in the stack, mirroring the way the product builds. Therefore roslyn and msbuild do not flow into runtime.

@oleksandr-didyk
Copy link
Contributor Author

oleksandr-didyk commented Feb 7, 2023

If I understood this correctly, this was for the product build, while in this task we are dealing with the repo build.

In product build runtime would utilize the PreviouslySourceBuildVersions for these two dependencies since they are not available (not source-built) at that time. In repo build, we can just pull the latest available versions from our feeds.

I might be wrong here so I'll ask for a confirmation from @MichaelSimons or @mmitche

@oleksandr-didyk
Copy link
Contributor Author

oleksandr-didyk commented Feb 7, 2023

Per offline discussion with Michael:

Viktor is correct about changed in build order regarding msbuild and nuget, I miss-understood the point Matt was making. We would need to create SBRPs for these two dependencies + the SBRPs would need to be updated by runtime if a new version would need to flow in. I can create them for the currently used versions (17.3.2 for msbuild and 6.2.2 for nuget)

Regarding TFM filtering: Michael mentioned a concern about the order of enabling TFM filtering, specifically that starting with runtime could cause issues.
As an example, say we filter out TFMs for a system library (e.g. System.Collections) and have it built only for latest TFM and netstandard. A repo that is then consuming this dependency (say, roslyn) is still building with the full TFM list, so when the source-build pulls in the new System.Collections it would fail the build.
As such, I will create a tracking issue for the pre-built TFMs and we can come back to it / discuss any future concerns once we have made more progress on it in other repos

@mmitche
Copy link
Member

mmitche commented Feb 7, 2023

I > In product build runtime would utilize the PreviouslySourceBuildVersions for these two dependencies since they are not available (not source-built) at that time. In repo build, we can just pull the latest available versions from our feeds.

This closely dovetails with a discussion we were having about runtime's dependency on roslyn. Runtime took advantage of a newer roslyn than appeared in any available SDK. Using the SDK tools would break the build, and there was no way to update the input SDK to a new enough SDK that would support the build.

IMO:

  • For actively iterating of tooling dependencies, we should allow repos to use the versions they wish during earlier stages of development. These would go in the baseline in the prebuilt report.
  • When we need to lock down for a release, we should be able to do so at the product build level, the tooling should be coming from the SDK.

@mmitche
Copy link
Member

mmitche commented Feb 7, 2023

Viktor is correct about changed in build order regarding msbuild and nuget, I miss-understood the point Matt was making. We would need to create SBRPs for these two dependencies + the SBRPs would need to be updated by runtime if a new version would need to flow in. I can create them for the currently used versions (17.3.2 for msbuild and 6.2.2 for nuget)

@oleksandr-didyk Depending on how these dependencies are used, they may not be SBRP-able (if they are tooling), and it may not make sense to SBRP them anyway if they are changing over time.

@adamsitnik
Copy link
Member

from trying out the latest version, it seems that for this package to be bumped source changes are required, as the package's API changed

If updating System.CommandLine version is blocking please let me know, I will prioritize updating all the repos (we are introducing breaking changes daily as we are polishing S.CL before it becomes a part of BCL)

@ViktorHofer
Copy link
Member

ViktorHofer commented Feb 8, 2023

@oleksandr-didyk Depending on how these dependencies are used, they may not be SBRP-able (if they are tooling), and it may not make sense to SBRP them anyway if they are changing over time.

@mmitche what's the alternative given that runtime builds before msbuild, nuget or commandline? Is there an alternative that would avoid publishing them to SBRP?

@mmitche
Copy link
Member

mmitche commented Feb 8, 2023

It may be that System.CommandLine needs to build before runtime. Thoughts @MichaelSimons? IIRC it's a leaf node in the graph so it can build whenever. That would make the most sense for System.CommandLine since it's changing over time.

I took another look at Microsoft.Build and NuGet.ProjectModel. I think those should be SBRPs (or, like @ViktorHofer mentions, we could change over to a version we already have an SBRP for). They're just API surface area.

I was confusing those with cases like roslyn, where runtime was using the tooling packages to run against newer functionality. Those are not SBRP-able.

@MichaelSimons
Copy link
Member

It may be that System.CommandLine needs to build before runtime.

S.CL already builds before runtime. Runtime should have a dependency flow for it.

@adamsitnik
Copy link
Member

We are also debating whether S.CL should become a part of dotnet/runtime or not. Would it help in this case? (so far I am gathering a list of all pros and cons)

@mmitche
Copy link
Member

mmitche commented Feb 8, 2023

@adamsitnik Depending on the stability of the API, I think it may hurt more than help. While the API is rapidly iterating, that would bring a few repos into have a direct dependency on runtime that did not have previously have one. That would mean it would be more difficult to get a coherent product.

Once it ships and has a stable API surface area, it's possible that we could SBRP S.CL and things would get simpler.

@oleksandr-didyk
Copy link
Contributor Author

Update on the status:

  • creating SBRPs for discussed packages (MSBuild, NuGet, etc) to see if they can even be used within runtime (i.e. if they are not tooling)
  • once the SBRPs are prepared -> testing changes made by source-building the product through the VMR using the repo PVP flow to verify SBRP usage

@oleksandr-didyk
Copy link
Contributor Author

Update on status:

  • created SBRP for MSBuild, runtime build with the SBRP was successful
  • moving on to other packages required to build the repo

@oleksandr-didyk
Copy link
Contributor Author

Update on status:

@oleksandr-didyk oleksandr-didyk moved this from In Progress to Blocked in .NET Source Build Apr 24, 2023
@oleksandr-didyk
Copy link
Contributor Author

Update on status:

@github-project-automation github-project-automation bot moved this from Blocked to Done in .NET Source Build May 18, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label May 18, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Jun 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

6 participants