search

dotnet / arcade

Tools that provide common build infrastructure for multiple .NET Foundation projects.
MIT License
673 stars 347 forks source link

Improve automated branch codeflow to avoid having to merge eng\* changes #10212

Open marcpopMSFT opened 2 years ago

marcpopMSFT commented 2 years ago

The SDK in the past has not used automated codeflow as it typically has merge conflicts in the eng* folder. Often these conflicts are related to the nuget.config file, versions.props, and versions.details.xml file.

Starting in .net 6, source build was heavily focused on 6.0.1xx and requested automated codeflow up to main so we've had that going for the past 9 month. We trained a vendor to resolve the merge conflicts but always accepting the higher branch code for conflicts in the eng* folder. This has worked reasonably well but source build has requested we continue having codeflow for .net 6 and .net 7 branches.

That means eventually we could have a change from 6.0.1xx that has to flow to 6.0.3xx, 6.0.4xx, 7.0.1xx, 7.0.3xx, 7.0.4xx, and then main having likely merge conflicts at each stage. That makes this pain rise to the level of filing this issue.

https://github.com/dotnet/sdk/pulls?q=is%3Apr+author%3Adotnet-maestro-bot https://github.com/dotnet/installer/pulls?q=is%3Apr+author%3Adotnet-maestro-bot

marcpopMSFT commented 2 years ago

Another reason why this is causing more pain as source build wants to support the 1xx releases for the life of the runtime. While that is still being discussed, that means we'd have more branches than we'd have in the past for each release staying in support (normally 1xx would go out of support when VS did).

mmitche commented 2 years ago

@marcpopMSFT I think this is worth tackling, but it is one of those things where we need to be able to crisply be able to define what the right thing to do is, and have the logic be transparent about what it will do. I think this will be difficult. Consider the following cases:

This is a general case of inter-branch merging should be very smart (https://github.com/dotnet/arcade/issues/4848) and know what we want it to do. But this is full of corner cases and requires a ton of product level context. If we could come up with a spec that details exactly what the action is in each case (and how we would detect it) then I would be supportive. It would also need to not get things wrong. One danger is that the bot gets 95% of cases but silently does the wrong thing for another 5%. Git catches most of these cases today with at least some merge conflicts popping out. If we eliminate those, we may not have a signal that something has gone wrong.

My hunch is that a case-by-case analysis is going to show the decision tree to be fairly complicated. That gives me less confidence that we've found all the cases.

mmitche commented 2 years ago

/cc @riarenas @adiaaida

marcpopMSFT commented 2 years ago

There is certainly risk of missing changes. For example, I realized to date that 6.0.4xx is on the 2.20 version of Applicationinsights but 7.0.1xx is on 2.19 partly because of a mistaken merge in the automated codeflow. Since that change was in the version.props file, if I had known it wasn't going to be automatically merged, I would have manually merged it up but since this issue hasn't been solved yet, I assumed that automated flow.

I think that just excluding a set of files from the merge might be better than what we have today which is a vendor merging as best he can.

mmitche commented 2 years ago

@marcpopMSFT That might not be the right thing to do though. aspnet, in the last release, turned off (or at least didn't merge) their release/7.0 subscriptions to arcade because they knew they'd get them through the rc1 branch. So in that case they'd get updated arcade dependencies but incompatible scripts.