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

Fix handling of wildcard AssemblyVersion properties during markup compilation #2691

Merged
merged 15 commits into from
Apr 1, 2020

Conversation

rladuca
Copy link
Member

@rladuca rladuca commented Mar 2, 2020

fixes #2517

@rladuca rladuca self-assigned this Mar 2, 2020
@ghost ghost requested a review from vatsan-madhavan March 2, 2020 23:24
@ghost ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Mar 2, 2020
@ghost ghost requested a review from SamBent March 2, 2020 23:24
@rladuca rladuca added this to the 5.0 milestone Mar 2, 2020
@lindexi
Copy link
Member

lindexi commented Mar 5, 2020

Maybe we should fix the line break.

@rladuca
Copy link
Member Author

rladuca commented Mar 5, 2020

Maybe we should fix the line break.

I think we need to sweep the repo and set some attributes to ensure we're all using the same line endings. I don't want to make single changes in this PR, for now I'll keep this as is and we'll make a choice and fixup the repo.

EDIT: The renormalization would be a bigger PR, so just going to fix this one file for now.

@rladuca rladuca changed the title Fix handling of wildcard AssemblyVersion properties during markup compilation WIP: Fix handling of wildcard AssemblyVersion properties during markup compilation Mar 6, 2020
Robert LaDuca added 3 commits March 6, 2020 14:08
… on the AssemblyVersion. This allows for the error to be thrown once per MarkupCompile pass and won't present itself as an error at some point in the XAML, but rather as a general error with the AssemblyVersion.
@rladuca rladuca changed the title WIP: Fix handling of wildcard AssemblyVersion properties during markup compilation Fix handling of wildcard AssemblyVersion properties during markup compilation Mar 12, 2020
@rladuca
Copy link
Member Author

rladuca commented Mar 12, 2020

Moving this from WIP to actual, I think it's working pretty well right now.

@rladuca rladuca requested a review from vatsan-madhavan March 13, 2020 01:07
@rladuca
Copy link
Member Author

rladuca commented Mar 31, 2020

I've checked the following scenarios:

  • Normal build (Deterministic + non-wildcard AssemblyVersion)
  • WildCard Framework-Style (Non-Deterministic + wildcard AssemblyVersionAttribute)
  • WildCard Core-Style (Non-Deterministic + wildcard AssemblyVersion)
  • Various permutations of bad AssemblyVersion properties (1.1.x, 1.1.1x, etc)

Everything works as expected. The error is shown on a bad AssemblyVersion (assuming XAML compilation occurs). Without the fix, wildcard applications using AssemblyVersion crash as in the report. With the fix, they no longer crash. Behavior is the same in both normal scenarios and in AssemblyVersionAttribute scenarios (as far as I can tell).

C# does give a warning about wildcard AssemblyVersion properties:

 obj\Debug\netcoreapp5.0\PbtWildCardTest.AssemblyInfo.cs(16,59): warning CS7035: The specified version string does not
 conform to the recommended format - major.minor.build.revision [d:\wpf\PbtWildCardTest\PbtWildCardTest.csproj]

That seems to be the only difference between using the attribute and the msbuild property.

@rladuca rladuca added the auto_merge bot-command label Apr 1, 2020
@ghost
Copy link

ghost commented Apr 1, 2020

Hello @rladuca!

Because this pull request has the auto_merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@rladuca rladuca removed the auto_merge bot-command label Apr 1, 2020
@rladuca rladuca merged commit 5a9976c into dotnet:master Apr 1, 2020
@HClausing
Copy link

Thanks @vatsan-madhavan !

Now I understand the correlation between these tags on csproj file and Application.LoadComponent() method.

@vatsan-madhavan
Copy link
Member

@dotnet/wpf-developers Please write a nice doc for this and make it discoverable through public documentation so that the next person can learn about it more easily. Right now the only way to discover it is for someone in the know to make the correlation and point community-members to this PR and the associated issue.

@ghost ghost locked as resolved and limited conversation to collaborators Apr 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
PR metadata: Label to tag PRs, to facilitate with triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

For non-deterministic builds WPF application crashes on resource dictionary load
5 participants