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

Annotate corefx build artifacts with commit hashes #40110

Open
eiriktsarpalis opened this issue Aug 7, 2019 · 9 comments

Comments

@eiriktsarpalis
Copy link
Member

commented Aug 7, 2019

I would be nice if assemblies produced by the corefx repo were annotated with git metadata. For example:

[assembly: AssemblyMetadata("GitBranch", "release/3.0")]
[assembly: AssemblyMetadata("GitHash", "abc123")]

Having this information available at runtime would help a great deal in traceability when investigating issues.

@ViktorHofer

This comment has been minimized.

Copy link
Member

commented Aug 7, 2019

cc @danmosemsft and @jkotas for opinions.

@jkotas

This comment has been minimized.

Copy link
Member

commented Aug 7, 2019

I think it is a fine idea. We used to do similar thing in .NET Core 2.1 (via unmanaged version resource). For example, .NET Core 2.1 has this in the unmanaged version resources: ProductVersion 4.6.27818.01 @BuiltBy: dlab14-DDVSOWINAGE054 @Branch: release/2.1 @SrcCode: https://github.com/dotnet/corefx/tree/637a8d58f72f2b0f1a71187530c3cf433e95a75a

.NET Core 3.0 has just the hash - without the branch and repo path: 3.0.0-preview8.19372.8+d594cb2dbc5027d7e996491b0722c5be5204460f

The question to answer is why we dropped it for .NET Core 3.0.

@ViktorHofer

This comment has been minimized.

Copy link
Member

commented Aug 7, 2019

pdbs now have sourcelink information embedded which should point to the commit, branch and repo path.

@danmosemsft

This comment has been minimized.

Copy link
Member

commented Aug 7, 2019

Yes please.

It would be nice to have in the IL as well. (Right now the version/hash is in both unmanaged and in the IL: [assembly: AssemblyInformationalVersion("3.0.0-preview8.19369.1+b05efe069ace397b0740f0bbedb393d19598cd73")]). as it's convenient when looking in ILspy, ildasm etc while the native resources are nice for Windows explorer, filever, etc.

@JamesNK

This comment has been minimized.

Copy link
Member

commented Aug 10, 2019

Yes please.

To go further, quite often external consumers of .NET are interested in knowing when a merged commit is going to be publicly available in an SDK. For example, me. gRPC is using .NET Core 3.0 and we are working through many HTTP/2 fixes that I want to double check. I have no idea whether an SDK has the fixes or not. Downloading the latest and inspecting an attribute would be an improvement, but it would be great if that info was available directly from the PR fix.

Could the build do something like automatically add metadata or comment to a PR in GitHub with a link to the first SDK build that will contain the fix? Alternatively, there could be a command invoked via a comment on the PR to do the same thing.

@danmosemsft

This comment has been minimized.

Copy link
Member

commented Aug 10, 2019

Could the build do something like automatically add metadata or comment to a PR in GitHub with a link to the first SDK build that will contain the fix

My first reaction is that this is not possible, since the build occurs befor we know which SDK will ingest it. But cc @stephentoub in case this will be come possible in the new repo world.

@JamesNK

This comment has been minimized.

Copy link
Member

commented Aug 10, 2019

The two stage build makes it difficult.

  1. The SDK build would need to be the one that adds metadata
  2. When it is queued a process would have to look at the framework versions it will include, see what commits have occurred in each since the last build
  3. Process would need to link the commits back to PRs in GitHub
  4. Then in GitHub you would add a comment/metadata to the PR about the queued SDK build

(my guess of how it could work, I'm not that familiar with the build system)

Pretty complicated 😬

@danmosemsft

This comment has been minimized.

Copy link
Member

commented Aug 10, 2019

Or low tech. Have a web app with the mapping. In the corefx libraries metadata have a URL to it containing the SHA in the query string. Voila you copy paste and go to that URL and it shows what SDK its in.

I'm now sure whether we plan to maintain such a page.

@am11

This comment has been minimized.

Copy link
Contributor

commented Aug 11, 2019

From dotnet.exe --info output, we get dotnet/core-setup SHA-1 (under the section useful for support), by which we can resolve CoreFX hash.

POSIX shell (requires xmlstarlet):

CoreSetupSha1=$(dotnet --info | grep.exe -A 3 'useful for support' | grep.exe 'Commit' | cut -d':' -f 2)
CoreFXSha1=$(curl -s https://raw.githubusercontent.com/dotnet/core-setup/${CoreSetupSha1##  }/eng/Version.Details.xml | xmlstarlet sel -t -m '//Dependency[@Name="Microsoft.NETCore.Platforms"]' -v 'Sha' -n)
echo "http://github.com/dotnet/corefx/commit/$CoreFXSha1"

PowerShell:

$CoreSetupSha1=$($(dotnet --info | Select-String 'useful for support' -Context 0,3).Context.PostContext | Select-String 'Commit').Line.Split(':')[1].Trim()
$CoreFXSha1=$([xml](Invoke-WebRequest https://raw.githubusercontent.com/dotnet/core-setup/$CoreSetupSha1/eng/Version.Details.xml).Content).SelectSingleNode('//Dependency[@Name="Microsoft.NETCore.Platforms"]').Sha
echo "http://github.com/dotnet/corefx/commit/$CoreFXSha1"

but it is dependent upon internal/implementation details; core-setup's release/2.x branches do not have Version.Details.xml file, but instead there is dependencies.props. Maybe it is best if dotnet --info flattens this information and lists all dependencies with SHA-1 hashes, to make it easier to discover and parse for the end-user and tools. If it is too verbose, it can use an additional switch, e.g. dotnet --info --hashes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.