Skip to content

Use RepoOriginalSourceRevisionId in ASP.NET Core #1572

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

BrennanConroy
Copy link
Member

@BrennanConroy BrennanConroy commented Jul 22, 2025

This should reenable us to have repo specific commit diffs for benchmarking regression/improvement analysis.

Will probably want to do the same in dotnet/runtime assuming this works.

Question: Do we only want to do this for nightlies? Should it be disabled for official builds?

Edit: Maybe we want OriginalRepoCommitUrl as well?
e.g. Today we have:

[assembly: AssemblyMetadata("CommitHash", "67889d9d2f21a890ac29bcd175c0d1937a401781")]
[assembly: AssemblyMetadata("SourceCommitUrl", "https://github.com/dotnet/dotnet/tree/67889d9d2f21a890ac29bcd175c0d1937a401781")]

@BrennanConroy
Copy link
Member Author

Verified it worked:
image

Copy link
Member

@wtgodbe wtgodbe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@ViktorHofer
Copy link
Member

ViktorHofer commented Jul 24, 2025

I'm not certain this information should get encoded into shipping artifacts. That SHA doesn't represent the shipping state. This information will be most certainly incorrect during servicing or in case of security fixes that are made directly against the VMR.

IOW this is an intermediate (an implementation detail about how we build the product) that shouldn't ship to customers.

Question: Do we only want to do this for nightlies? Should it be disabled for official builds?

That would be an option. Or only adding this in Debug / non-Release Configuration builds?

@akoeplinger
Copy link
Member

@ViktorHofer I don't agree. When we added this feature we discussed this and everyone is aware that it is not 100% correct at all times, but its still very useful to know what the origin is.

@BrennanConroy
Copy link
Member Author

Question: Do we only want to do this for nightlies? Should it be disabled for official builds?

That would be an option. Or only adding this in Debug / non-Release Configuration builds?

What we're trying to improve is how easy it is to investigate performance issues. Debug/non-release builds aren't used for benchmarking, nightlies are used. So that could be a good option if we want to avoid the metadata in official builds.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

4 participants