Runtime: Runtime repo consuming to jitUtils repo without using package or dependency update.

[Anipik]

We run a formatting task in our ci, which depends on jitutils tool.
We download the file https://github.com/dotnet/runtime/blob/master/src/coreclr/tests/scripts/format.py#L122

we then run this file to clone the entire repo https://github.com/dotnet/jitutils/blob/master/bootstrap.cmd#L82

We dont use any branches or updating mechanism for this dependency. We should use some other way to consume this dependency.

cc @dotnet/runtime-infrastructure

[safern]

cc: @erozenfeld @BruceForstall

[BruceForstall]

There have been some efforts in this direction: https://github.com/dotnet/jitutils/pull/219, https://github.com/dotnet/jitutils/pull/208. It's never been very high priority, given the implementation cost compared to maintaining the status quo.

cc @echesakovMSFT

[jashook]

This job only makes sense to run in the active development branch. I do not believe their is value in supporting it anywhere other than runtime/master. I would suggest closing this issue.

[ViktorHofer]

Are we running the job anywhere else than runtime master? I'm sure we run it in the 5.0 release branches.

[BruceForstall]

It probably gets run when we branch for releases, but we normally turn it off in servicing branches if/when it becomes a problem.

[ViktorHofer]

So this means we have a dependency problem here. Any cross-cutting changes would need to go through multiple branches, i.e. today: runtime/release/5.0-preview1, runtime/release/5.0-preview2, runtime/master and presumable some coreclr/release/3.x branches

[BruceForstall]

So this means we have a dependency problem here. Any cross-cutting changes would need to go through multiple branches, i.e. today: runtime/release/5.0-preview1, runtime/release/5.0-preview2, runtime/master and presumable some coreclr/release/3.x branches

I'm not quite sure I know what you mean here. If changes are required to jit-format in the jitutils branch to adjust for some dotnet/runtime change, that make it incompatible with the servicing branches, then we would need to address that, possibly by disabling the jit-format run in the servicing branch pipelines.

[ViktorHofer]

Right, that's the point. Changes which need to happen in jitutils first, aren't possible to be made unless you disable the job in all other branches except runtime/master which would then react to the change. Having a branching model protects from such fallouts, ie you forget about a branch which then starts to fail in CI.

[ViktorHofer]

The other disadvantage of the current consumption model is that the dependency doesn't show up in the graph:

cc @mmitche

[jaredpar]

The other disadvantage is that changes to the jitutils repo can unintentionally break the runtime tests. Because we grab from master vs. a specific commit that means changes propagate to this repository immediately.

Curious: why did we decide on this method of consuming jitutils vs. a submodule relationship? That seems like what we're effectively doing here just not using a submodule.

[BruceForstall]

Curious: why did we decide on this method of consuming jitutils vs. a submodule relationship?

I don't understand the implication.

Bottom-line: this system was created years ago, very early, and hasn't been updated since then. As stated above, there have been slight efforts to improve this situation.

[jaredpar]

I don't understand the implication.

Not trying to imply anything here. Was curious if you all considered doing this as a submodule instead of essentially downloading the repository during job execution. What you're doing seems very similar to having a submodule. Wanted to know if you all considered that as a solution.

[BruceForstall]

Sorry, when I said "I don't understand the implication", I really meant "I don't understand submodules" :-)

I'm sure nobody has considered that as an alternative.

[ViktorHofer]

I'm sure we aren't considering submodules at this point? When I spoke with others about this in the past we were strongly against using submodules for corefx/coreclr.

I still believe there is value in this discussion - onboarding to dependency flow would make sense IMO.

[jaredpar]

I'm sure we aren't considering submodules at this point? When I spoke with others about this in the past we were strongly against using submodules for corefx/coreclr.

I'm not saying we should use submodules. I just wanted to know if they were considered as an alternative. IF they had and were discarded I wanted to understand why. Sounds like they weren't considered though so a mute point.

[ericstj]

Silly alternative to a submodule: a nuget package published from JitUtils that contains a targets file that sets a property to the hash of the dependent repo. Could work in existing system and not impose new clone . dependency flow requirements. Just a thought, feel free to downvote.

[jashook]

As mentioned, in my opinion the action item to take away from this is to turn off this in every branch other than runtime/master

[ViktorHofer]

... and condition the step to only run on master to avoid future fallouts when we branch off from master.

[safern]

Something to consider when conditioning the job, the condition needs to be when branch or targetBranch is master so that it also runs on PRs against master.

[BruceForstall]

Maybe https://github.com/dotnet/runtime/pull/33896 will do it?

[ViktorHofer]

Closing as https://github.com/dotnet/runtime/pull/33896 as a workaround is good enough.