Remove reflection from FrameworkDescription#102164
Conversation
ghost
added
the
needs-area-label
dotnet-policy-service
bot
added
the
community-contribution
am11
added
area-System.Runtime
community-contribution
|
Tagging subscribers to this area: @dotnet/area-system-runtime |
| } | ||
| } | ||
|
|
||
| internal const string InternalVersion = "9.0.0"; |
There was a problem hiding this comment.
I do not think we want to add yet another place to update for version bumps. If you want to do this, it needs to be wired to eng/Versions.props.
There was a problem hiding this comment.
The version update bump looks like this 96c2e1d once a year. Is it really a big deal to update a string? If you think it is, lets close this PR then. It was a simple improvement with no visible effect on production bits.
There was a problem hiding this comment.
The version bumps are done every month in servicing, and they look like this: #101779
There was a problem hiding this comment.
Ah, the patch version. 🤭
Let me think a bit how to dedup it.
|
|
||
| Assert.DoesNotContain("+", version); // no git hash | ||
|
|
||
| #if STABILIZE_PACKAGE_VERSION |
There was a problem hiding this comment.
I think this should stay as is. It is a regression test for a bug that slipped through the cracks some time ago.
am11
force-pushed
the
feature/versioning/cleanups
branch
from
4c08f94 to
db1420d
Compare
src/libraries/System.Private.CoreLib/src/System/Runtime/InteropServices/RuntimeInformation.cs
Outdated
Show resolved
Hide resolved
...ices.RuntimeInformation.Tests/System.Runtime.InteropServices.RuntimeInformation.Tests.csproj
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems
Outdated
Show resolved
Hide resolved
am11
force-pushed
the
feature/versioning/cleanups
branch
2 times, most recently
from
80d8f1f to
791a6e7
Compare
am11
force-pushed
the
feature/versioning/cleanups
branch
from
791a6e7 to
c1ce415
Compare
| @@ -5,6 +5,7 @@ | |||
| </PropertyGroup> | |||
There was a problem hiding this comment.
Side note, <TargetFramework>netstandard2.0</TargetFramework> this can probably use $(NetCoreAppCurrent) as it is only used by corelib building for current version.
There was a problem hiding this comment.
There may be cases where the generator runs on .NET Framework if people open the project in Visual Studio.
|
Tested with $ ls artifacts/obj/*version*
artifacts/obj/_version.c artifacts/obj/_version.h artifacts/obj/runtime_version.h |
src/libraries/System.Private.CoreLib/gen/ProductVersionInfoGenerator.cs
Outdated
Show resolved
Hide resolved
…erator.cs Co-authored-by: Jan Kotas <jkotas@microsoft.com>
That's expected. Different CoreLib builds should not be sharing files. |
| { | ||
| context.RegisterPostInitializationOutput(ctx => | ||
| { | ||
| string? informationalVersion = typeof(ProductVersionInfoGenerator).Assembly.GetCustomAttribute<AssemblyInformationalVersionAttribute>()?.InformationalVersion; |
There was a problem hiding this comment.
I'm not sure how our build system handles this. Is this guaranteed to always match what typeof(object).Assembly would in the built corelib?
There was a problem hiding this comment.
As far as I can tell, the build system stamps the exact same AssemblyInformationalVersionAttribute on all binaries.
* Remove reflection from FrameworkDescription * Switch to source gen * Address CR feedback * Update src/libraries/System.Private.CoreLib/gen/ProductVersionInfoGenerator.cs Co-authored-by: Jan Kotas <jkotas@microsoft.com> --------- Co-authored-by: Jan Kotas <jkotas@microsoft.com>
3 participants
Tiny help for trimming; shaved 2KB off of
artifacts/bin/coreclr/osx.arm64.Release/System.Private.CoreLib.dll.