Include VMR version in productCommit

[am11]

Follow up on #5030.

$ curl -sSL https://aka.ms/dotnet/10.0.1xx/daily/productCommit-linux-x64.json | jq

{
  "runtime": {
    "commit": "ad8565092bbfdd5c8b4a94a718d10b2d394f7aee",
    "version": "10.0.0-preview.5.25265.101"
  },
  "aspnetcore": {
    "commit": "ad8565092bbfdd5c8b4a94a718d10b2d394f7aee",
    "version": "10.0.0-preview.5.25265.101"
  },
  "windowsdesktop": {
    "commit": "ad8565092bbfdd5c8b4a94a718d10b2d394f7aee",
    "version": "10.0.0-preview.5.25265.101"
  },
  "sdk": {
    "commit": "5a4292947487a9d34f4256c1d17fb3dc26859174",
    "version": "10.0.100-preview.5.25266.23"
  }
}

Ideally it should include the VMR commit and missing repos which VMR built beyond these four.

[ViktorHofer]

These should already all be the VMR commit and be the same value: https://github.com/dotnet/sdk/blob/35f0e6260522d17c4c2812107a4ce48ba89b18bf/src/Layout/redist/targets/GenerateBundledVersions.targets#L18-L61

Isn't that the case?

[ViktorHofer]

Could it be that this file is from an SDK produced out of dotnet/sdk? That repo still publishes their official builds.

[am11]

Yup, it's from https://github.com/dotnet/sdk/blob/35f0e6260522d17c4c2812107a4ce48ba89b18bf/src/Tasks/sdk-tasks/GetDependencyInfo.cs (click on any badge in first column and change .txt to .json). Ideally, it should contain version info of VMR and each constituent repos so we can validate versions without traversing multiple places.

[ViktorHofer]

Ideally, it should contain version info of VMR and each constituent repos so we can validate versions without traversing multiple places.

Sdk, windowsdesktop, aspnetcore and runtime all get built from within the PR so the VMR SHA is the only correct information here. I don't we should link back to the original repo SHA here as nothing guarantees that those are in sync. I.e. we directly make changes to the VMR that affects these repos.

For that same reason, the sourcelink SHA in nuget packages produced out of the VMR all link exclusively to the VMR commit.

[am11]

we directly make changes to the VMR that affects these repos.

Are we making changes just in eng/ or src/ of constituent repos as well? In case of latter, it kind of breaks the contract of productCommit-<os>-<arch>.json as we can't trace back to https://github.com/dotnet/<repo>/tree/<hash> or the actual commit which made the change we are interested in. If each commit repo should have its own valid commit, that would make much more sense to link to source.

Otherwise, VMR's /dotnet/artifacts/staging/assets/Release/Sdk/{version}/productCommit-{os}-{arch}.json should only contain one item "dotnet" with commit and version of that repo, navigatable with https://github.com/dotnet/dotnet/tree/{commit}.

[ViktorHofer]

Otherwise, VMR's /dotnet/artifacts/staging/assets/Release/Sdk/{version}/productCommit-{os}-{arch}.json should only contain one item "dotnet" with commit and version of that repo, navigatable with https://github.com/dotnet/dotnet/tree/{commit}.

That's the plan, yes. At the moment, all entries point to the same SHA and version for the VMR build. So this is just clean-up that we haven't started doing yet as we still have the repo official builds running. We shouldn't change this yet.

[am11]

Back to the original issue; missing repos in productCommit.

All VMR repos: arcade, aspire, aspnetcore, cecil, command-line-api, deployment-tools, diagnostics, efcore, emsdk, fsharp, msbuild, nuget-client, razor, roslyn, roslyn-analyzers, runtime, scenario-tests, sdk, source-build-externals, source-build-reference-packages, sourcelink, symreader, templating, vstest, windowsdesktop, winforms, wpf, xdt

and product commit only include four:

$ git clone --single-branch --depth 1 -b main https://github.com/dotnet/dotnet
$ cd dotnet
$ ./build.sh --clean-while-building --prep -sb --os linux --arch riscv64

$ cat artifacts/staging/assets/Release/Sdk/10.0.100-preview.5/productCommit-linux-riscv64.json
{
  "runtime": { "commit": "", "version": "10.0.0-dev" },
  "aspnetcore": { "commit": "", "version": "10.0.0-dev" },
  "windowsdesktop": { "commit": "", "version": "10.0.0-preview.5.25263.108" },
  "sdk": { "commit": "", "version": "10.0.100-dev" }
}

Should it include the rest?

Note commit is missing and windowsdesktop is not aligned.

[ViktorHofer]

Should it include the rest?

No, that's explicitly an anti goal. The VMR is not just a projection of the underlying repositories. It's the source of the .NET SDK that ships to customers. While the underlying repositories exist and sync into the VMR; for product composition only the VMR matters.

The productCommit.json files aren't shipping inside the .NET SDK. IIRC they are just a support file for our publishing or for the dotnet-install script. I don't think that file needs to get produced in source-build.

[am11]

Having four repos but not the rest is hard to make sense.

Having commit info of all VMR constituent repos makes most sense.

Having commit info of one repo "dotnet" in VMR also makes sense, but then we will need to traverse VMR, blame the which change updated the code, find that PR, browse to the original repo to get to the original change when investigating stuff. This defeats the purpose of this file..

IIRC they are just a support file for our publishing or for the dotnet-install script.

They are used by other tools like crank (and also godbolt?). I use them to find the source of truth; when/why was the change made under investigation etc.

[ViktorHofer]

Having commit info of one repo "dotnet" in VMR also makes sense, but then we will need to traverse VMR, blame the which change updated the code, find that PR, browse to the original repo to get to the original change when investigating stuff. This defeats the purpose of this file..

We need to do that anyway. I.e. for servicing (and really already now in main) we plan to make many changes directly in the VMR. Only the VMR's commit is the correct data.

Having four repos but not the rest is hard to make sense.

I agree which I think we should either remove this file entirely or replace the content with just one line for the VMR and its SHA.

[mmitche]

The productCommit* files were originally meant as a way to track the logical product components back to source. For example, they've never differentiated between windowsdesktop, winforms, and wpf. It's just been the sha of windowsdesktop, where the specific asset is produced.

Remember that the VMR is not always an exact copy of what is in the repo. The VMR could be "ahead" in the sense that it has source changes that have not made it back into the repo.

If you went a more mechanical route, building a graph, including a productCommit for everything that contributes, then it's hard to know where you'd "stop" building such a graph, or what you'd do about incoherency. There are numerous versions of components that aren't in the VMR that end up in the SDK. A few versions of roslyn, for instance.

Maybe something like?

{
  "dotnet": {
    "commit": "ad8565092bbfdd5c8b4a94a718d10b2d394f7aee",
    "version": "10.0.0-preview.5.25265.101"
    "lastFlow" : [
      "runtime": "abcdef",
      "arcade": "1234123123"
      ...
    }
  },
}

Of course, such a change is breaking compared to downlevel versions, so we would need to be wary.

[am11]

such a change is breaking

A single level lookup like vmr/src/source-manifest.json#repositories would be enough to answer "code in SDK tarball is based on these commits and versions". I wasn't expecting any breaking change:

- existing
+ additions

{
  "runtime": { "commit": "", "version": "10.0.0-dev" },
  "aspnetcore": { "commit": "", "version": "10.0.0-dev" },
  "windowsdesktop": { "commit": "", "version": "10.0.0-preview.5.25263.108" },
  "sdk": { "commit": "", "version": "10.0.100-dev" },
-  "sdk": { "commit": "", "version": "10.0.100-dev" }
+  "sdk": { "commit": "", "version": "10.0.100-dev" },
+  "dotnet": { "commit": "", "version": "10.0.0-dev" },
+  "arcade": { "commit": "", "version": "10.0.0-dev" },
+  "aspire": { "commit": "", "version": "10.0.0-dev" },
...
}

[ViktorHofer]

I'm not sure you are acknowledging what we wrote above. The VMR's source is based on its inner repos but moves ahead at any time. I strong believe that we shouldn't point to a SHA that doesn't represent the shipping product. That will result in debugging hell.

At the end of the day what counts is:

• dotnet/dotnet's SHA is the only information that correctly connects the VMR outputs to the sources. That SHA is part of a package's nuspec sourcelink information.
• From the product's perspective, the SHAs of the inner repos don't matter at all. They only exist for us to know the SHA of the last sync. That's not information that should be relevant to customers.
• Listing the inner repos is leaking implementation details.

[am11]

I'm not sure you are acknowledging what we wrote above.

I commented on the breaking change bit, and there's no contradiction in what I said.

That's not information that should be relevant to customers.

As I mentioned above, the productCommit file is used outside of the installer script. it's linked from the SDK table and has existed for a long time. Over time, it has gained multiple external uses, which is why this issue focuses specifically on that file; no graph, just flat list of version infos. It doesn't have to capture the whole world either.