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.

Sign up for GitHub

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

Jump to bottom

Microsoft.Extensions.Logging.Generators.Tests hardcode the AssemblyVersion #58175

Closed
ViktorHofer opened this issue Aug 26, 2021 · 8 comments · Fixed by #58220
Closed

Microsoft.Extensions.Logging.Generators.Tests hardcode the AssemblyVersion #58175

ViktorHofer opened this issue Aug 26, 2021 · 8 comments · Fixed by #58220
Assignees
@allantargino
Labels
area-Extensions-Logging good first issue Issue should be easy to implement, good for first-time contributors help wanted [up-for-grabs] Good issue for external contributors
Milestone
6.0.0

Comments

@ViktorHofer
Copy link
Member

ViktorHofer commented Aug 26, 2021 •
edited
Loading

The generated logging generator baseline files all hardcode the generator's assembly version, i.e.:

runtime/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/Baselines/TestWithTwoParams.generated.txt

Line 8 in 6d5a772

[global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "6.0.0.0")]

This makes reving the repository to the next major version (including tfm and assembly version) harder as it needs to be, as the test files need to be updated whenever assembly versions change.

@maryamariyan can you please look into this so that we can avoid the hardcoded version?

Failed in #58011.
@dotnet-issue-labeler dotnet-issue-labeler bot added area-Infrastructure-libraries untriaged New issue has not been triaged by the area owner labels Aug 26, 2021
@ghost
Copy link

ghost commented Aug 26, 2021

Tagging subscribers to this area: @Anipik, @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

Issue Details

The generated logging generator baseline files all hardcode the generator's assembly version, i.e.:

runtime/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/Baselines/TestWithTwoParams.generated.txt

Line 8 in 6d5a772

[global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "6.0.0.0")]
.

This makes reving the repository to the next major version (including tfm and assembly version) hard as the test files need to updated with it.

@maryamariyan can you please look into this so that we can avoid the hardcoded version?

Failed in #58011.

Author: ViktorHofer
Assignees: -
Labels:

area-Infrastructure-libraries, untriaged
Milestone: -

@ghost
Copy link

ghost commented Aug 26, 2021

Tagging subscribers to this area: @maryamariyan
See info in area-owners.md if you want to be subscribed.

Issue Details

The generated logging generator baseline files all hardcode the generator's assembly version, i.e.:

runtime/src/libraries/Microsoft.Extensions.Logging.Abstractions/tests/Microsoft.Extensions.Logging.Generators.Tests/Baselines/TestWithTwoParams.generated.txt

Line 8 in 6d5a772

[global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Logging.Generators", "6.0.0.0")]
.

This makes reving the repository to the next major version (including tfm and assembly version) hard as the test files need to updated with it

@maryamariyan can you please look into this so that we can avoid the hardcoded version?

Failed in #58011.

Author: ViktorHofer
Assignees: -
Labels:

untriaged, area-Extensions-Logging
Milestone: -

@maryamariyan maryamariyan removed the untriaged New issue has not been triaged by the area owner label Aug 26, 2021
@maryamariyan maryamariyan added this to the 6.0.0 milestone Aug 26, 2021
@maryamariyan
Copy link
Member

@ViktorHofer what's your recommendation for the fix here?

@sharwell
Copy link
Member

  • When generating *.generated.txt files, replace the current version number (6.0.0.0 above) with a literal like %VERSION%
  • When reading *.generated.txt files, replace %VERSION% with the effective version of the source generator assembly

@ViktorHofer
Copy link
Member Author

Exactly what Sam said. Use replacement tokens instead of the hardcoded versions.

@maryamariyan maryamariyan added good first issue Issue should be easy to implement, good for first-time contributors help wanted [up-for-grabs] Good issue for external contributors labels Aug 26, 2021
@allantargino
Copy link
Contributor

I would like to take this one :)

allantargino added a commit to allantargino/dotnet-runtime that referenced this issue Aug 26, 2021
@allantargino
Replacement tokens in logging test baseline files
a175f82 
Instead of using a hardcoded version of the assembly version
on the logging generated baseline files used by tests we use
replacement tokens and at runtime we replace them.

fix dotnet#58175
@allantargino allantargino mentioned this issue Aug 26, 2021
Merged
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 26, 2021
ViktorHofer pushed a commit that referenced this issue Aug 27, 2021
@allantargino
Replacement tokens in logging test baseline files (#58220)
126be90 
* Replacement tokens in logging test baseline files

Instead of using a hardcoded version of the assembly version
on the logging generated baseline files used by tests we use
replacement tokens and at runtime we replace them.

fixes #58175
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 27, 2021
@sharwell
Copy link
Member

sharwell commented Aug 27, 2021 •
edited
Loading

It appears that the version number of this assembly is locked at 6.0.0.0. This can be problematic for VBCSCompiler during reference package updates, leading to dramatic reduction in compiler throughput (for Roslyn.sln, builds are about 3x slower). The fix for this solution is ensuring the assembly version number increases with each build for analyzer and source generator packages.

@ViktorHofer
Copy link
Member Author

@sharwell can you please file a new issue for your concern? also cc @ericstj and @Anipik for incremental servicing.

@sharwell sharwell mentioned this issue Aug 27, 2021
Closed
github-actions bot pushed a commit that referenced this issue Sep 10, 2021
@allantargino
Replacement tokens in logging test baseline files
8d0e7fc 
Instead of using a hardcoded version of the assembly version
on the logging generated baseline files used by tests we use
replacement tokens and at runtime we replace them.

fix #58175
Anipik pushed a commit that referenced this issue Sep 15, 2021
@github-actions @allantargino
[release/6.0] Increment source generator assembly versions with every…
c705b70 
… build (#58919)

* increment source generator assembly versions with every build

* use isGenerator Project property

* add comment

* Replacement tokens in logging test baseline files (#58220)

* Replacement tokens in logging test baseline files

Instead of using a hardcoded version of the assembly version
on the logging generated baseline files used by tests we use
replacement tokens and at runtime we replace them.

fixes #58175

Co-authored-by: Anirudh Agnihotry <anagniho@microsoft.com>
Co-authored-by: Allan Targino <13934447+allantargino@users.noreply.github.com>
@ghost ghost locked as resolved and limited conversation to collaborators Sep 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@allantargino allantargino

Labels
area-Extensions-Logging good first issue Issue should be easy to implement, good for first-time contributors help wanted [up-for-grabs] Good issue for external contributors
Projects
None yet
Milestone
6.0.0
Development

Successfully merging a pull request may close this issue.

Replacement tokens in logging test baseline files allantargino/dotnet-runtime
4 participants
@sharwell @maryamariyan @ViktorHofer @allantargino