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

Use generated version files when printing/tracing host version info #90273

Merged
merged 3 commits into from
Aug 11, 2023

Conversation

elinor-fung
Copy link
Member

@elinor-fung elinor-fung commented Aug 9, 2023

Use generated version files when printing/tracing host version info.

  • Use VER_PRODUCTVERSION_STR / sccsid from generated _version.(h|c) for version and commit info (for tracing)
  • Use RuntimeProductVersion for version from header generated by GenerateRuntimeVersionFile
  • Remove HOST_*_PKG_VER defines and script arguments for passing that info around

Fixes #80961

Host traces will have:

--- Invoked apphost [version: 8.0.0-dev @Commit: c088228be355eef650fbde9593122e21b2b124aa] main = {
--- Invoked hostfxr_main_bundle_startupinfo [version: 8.0.0-dev @Commit: c088228be355eef650fbde9593122e21b2b124aa]
--- Invoked hostpolicy [version: 8.0.0-dev @Commit: c088228be355eef650fbde9593122e21b2b124aa] corehost_main = {

@ghost
Copy link

ghost commented Aug 9, 2023

Tagging subscribers to this area: @agocke, @vitek-karas, @VSadov
See info in area-owners.md if you want to be subscribed.

Issue Details

Set REPO_COMMIT_HASH and HOST_*_VER defines when building runtime - which builds singlefilehost. This makes it so that we don't completely lose the version information for single-file host and host traces will have:

--- Invoked apphost [version: 8.0.0-dev, commit hash: 47f82ef6996d08735a12444409f6c9881605a8d9] main = {
--- Invoked hostfxr_main_bundle_startupinfo [version: 8.0.0-dev, commit hash: 47f82ef6996d08735a12444409f6c9881605a8d9]
--- Invoked hostpolicy [version: 8.0.0-dev, commit hash: 47f82ef6996d08735a12444409f6c9881605a8d9] corehost_main = {

Fixes #80961

There's more cleanup that can be done here (for example, we no longer need all the separate defines/versions for each host component - all the versions are the same), but this just fixes the immediate issue of single-file not having version/commit info at all.

Author: elinor-fung
Assignees: -
Labels:

area-Single-File

Milestone: -

@elinor-fung elinor-fung requested a review from VSadov August 9, 2023 21:27
@am11
Copy link
Member

am11 commented Aug 9, 2023

_version.c is generated here:

substitute="$(printf 'static char sccsid[] __attribute__((used)) = "@(#)Version N/A @Commit: %s";\n' "$commit")"
and included in multiple (all native?) projects, including the hosts:
list(APPEND SOURCES ${VERSION_FILE_PATH})

would be nice to generalize and reuse that global info instead of wiring up a new mechanism.

e.g. diagnostics has mirrored eng/native directory, and we reused that SCCSID thing in SOS which also needed it internally to log the version and commit info, with a small cmake hack: https://github.com/dotnet/diagnostics/blob/ef7b8a56a162818e0af8bf5fe1de589fa0bf38e4/CMakeLists.txt#L41-L46.

All these seem very similar use-cases for which we can do something better, IMO.

Currently, one limitation is imposed by arcade is that it only allows one version file per platform, so it is either _version.c (Unix) or _version.h (Windows): https://github.com/dotnet/arcade/blob/main/src/Microsoft.DotNet.Arcade.Sdk/tools/Version.targets., we can change it to emit both on all platforms.

cc @jkoritzinsky, @ViktorHofer

@elinor-fung elinor-fung changed the title Set defines for version and commit when building single-file host Use generated version files when printing/tracing host version info Aug 10, 2023
@elinor-fung
Copy link
Member Author

Thanks. Switched to using generated _version.h/version.h and runtime_verion.h and removed the assorted version defines in the host.

Copy link
Member

@am11 am11 left a comment

Choose a reason for hiding this comment

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

Looks great!

These properties can also be cleaned up:

<!--
By default, we are always building the nuget packages for HostPolicy, HostFXR and
Dotnet/AppHost. Thus, the properties (below) are always set to $(ProductVersion).
However, there are scenarios when only some of these components will change (e.g. during
servicing, we may only change HostPolicy but not HostFXR and Dotnet/AppHost). In such cases,
pass the appropriate version value(s) as argument to the build command in order to override;
e.g. 'build -p:HostPolicyVersion=x.y.z ...'
-->
<HostVersion Condition="'$(HostVersion)' == ''">$(ProductVersion)</HostVersion>
<AppHostVersion Condition="'$(AppHostVersion)' == ''">$(ProductVersion)</AppHostVersion>
<HostResolverVersion Condition="'$(HostResolverVersion)' == ''">$(ProductVersion)</HostResolverVersion>
<HostPolicyVersion Condition="'$(HostPolicyVersion)' == ''">$(ProductVersion)</HostPolicyVersion>

@elinor-fung
Copy link
Member Author

I think the only things still using those are the host packages (which I opted not to touch in this change). Most of those can be removed (and the remaining one switched to just used the product version) when we get to #35244

@elinor-fung elinor-fung merged commit 77624c1 into dotnet:main Aug 11, 2023
170 checks passed
@elinor-fung elinor-fung deleted the single-file-version branch August 11, 2023 19:03
@ghost ghost locked as resolved and limited conversation to collaborators Sep 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Single-file host tracing should include version and/or commit hash
3 participants