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

For non-deterministic builds WPF application crashes on resource dictionary load #2517

Closed
the-black-wolf opened this issue Feb 7, 2020 · 24 comments · Fixed by #2691
Closed
Assignees
Labels
Bug Product bug (most likely)
Milestone

Comments

@the-black-wolf
Copy link

If project is set to be non-deterministic and AssemblyVersion uses wildcards, the application fails while loading resource dictionaries. It seem to try to parse Version object from wildcard string from meta (?) instead of determining concrete running assembly version, either that or there is a bug in Version parser.
Note: I understand there are benefits to deterministic, but that is not the topic, non-deterministic should not be a showstopper for wpf core, especially when porting existing applications and follow-up processes.

Steps to reproduce:

  1. Create .net core WPF application
  2. Modify csproj and switch to wildcard non-deterministic build
<Project Sdk="Microsoft.NET.Sdk.WindowsDesktop">
  <PropertyGroup>
    <OutputType>WinExe</OutputType>
    <TargetFramework>netcoreapp3.1</TargetFramework>
    <UseWPF>true</UseWPF>
    <!-- non-deterministic with wildcards -->
    <Deterministic>False</Deterministic>
    <AssemblyVersion>1.0.*</AssemblyVersion>
  </PropertyGroup>
</Project>
  1. Add any resource file to project (e.g. test.xaml), contents is irrelevant.
  2. Load resource from App.xaml
<Application x:Class="WpfApp1.App"
             xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
             xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
             xmlns:local="clr-namespace:WpfApp1"
             StartupUri="MainWindow.xaml">
    <Application.Resources>
        <ResourceDictionary>
            <ResourceDictionary.MergedDictionaries>
                <ResourceDictionary Source="Test.xaml" />
            </ResourceDictionary.MergedDictionaries>
        </ResourceDictionary>
    </Application.Resources>
</Application>
  1. Build&Run app

Expected behavior
A working application?

Actual behavior
Application crashes with unhandled exception immediately on start. Stack trace is:

Application: WpfApp1.exe
CoreCLR Version: 4.700.19.56402
.NET Core Version: 3.1.0
Description: The process was terminated due to an unhandled exception.
Exception Info: System.FormatException: Input string was not in a correct format.
   at System.Number.ThrowOverflowOrFormatException(ParsingStatus status, TypeCode type)
   at System.Version.TryParseComponent(ReadOnlySpan`1 component, String componentName, Boolean throwOnFailure, Int32& parsedComponent)
   at System.Version.ParseVersion(ReadOnlySpan`1 input, Boolean throwOnFailure)
   at System.Version.Parse(String input)
   at System.Version..ctor(String version)
   at System.Windows.Navigation.BaseUriHelper.GetLoadedAssembly(String assemblyName, String assemblyVersion, String assemblyKey)
   at MS.Internal.AppModel.ResourceContainer.GetResourceManagerWrapper(Uri uri, String& partName, Boolean& isContentFile)
   at MS.Internal.AppModel.ResourceContainer.GetPartCore(Uri uri)
   at System.IO.Packaging.Package.GetPartHelper(Uri partUri)
   at System.IO.Packaging.Package.GetPart(Uri partUri)
   at System.Windows.Application.GetResourceOrContentPart(Uri uri)
   at System.Windows.Application.LoadComponent(Object component, Uri resourceLocator)
   at WpfApp1.App.InitializeComponent() in C:\playground\WpfApp1\WpfApp1\App.xaml:line 1
   at WpfApp1.App.Main()
@rladuca rladuca added the Bug Product bug (most likely) label Feb 11, 2020
@rladuca rladuca added this to the 5.0 milestone Feb 11, 2020
@vatsan-madhavan
Copy link
Member

I can reproduce this. When using msbuild, i get these warnings:

obj\Debug\netcoreapp3.1\test14_2yzylepx_wpftmp.AssemblyInfo.cs(16,59): warning CS7035: The specified version string does not conform to the recommended format - major.minor.build.revision [C:\Temp\wpf\test14\test14_2yzylepx_wpftmp.csproj]

obj\Debug\netcoreapp3.1\test14.AssemblyInfo.cs(16,59): warning CS7035: The specified version string does not conform to the recommended format - major.minor.build.revision [C:\Temp\wpf\test14\test14.csproj]

In Visual Studio, I get the same warnings:

1>C:\Temp\wpf\test14\obj\Debug\netcoreapp3.1\test14_1y2fwphz_wpftmp.AssemblyInfo.cs(16,59,16,66): warning CS7035: The specified version string does not conform to the recommended format - major.minor.build.revision
1>Done building project "test14_1y2fwphz_wpftmp.csproj".
1>C:\Temp\wpf\test14\obj\Debug\netcoreapp3.1\test14.AssemblyInfo.cs(16,59,16,66): warning CS7035: The specified version string does not conform to the recommended format - major.minor.build.revision
1>test14 -> C:\Temp\wpf\test14\bin\Debug\netcoreapp3.1\test14.dll

dotnet build also gives the same warnings:

obj\Debug\netcoreapp3.1\test14_xoo2scrw_wpftmp.AssemblyInfo.cs(15,59): warning CS7035: The specified version string does not conform to the recommended format - major.minor.build.revision [C:\Temp\wpf\test14\test14_xoo2scrw_wpftmp.csproj]
obj\Debug\netcoreapp3.1\test14.AssemblyInfo.cs(16,59): warning CS7035: The specified version string does not conform to the recommended format - major.minor.build.revision [C:\Temp\wpf\test14\test14.csproj]

Running the exe crashes exactly how @the-black-wolf describes. but running it using dotnet run is more illuminating.

I removed <Deterministic>false</Deterministic> from the project, and ran it like this:

dotnet run /p:Deterministic=false

This is what I got:

obj\Debug\netcoreapp3.1\test14_10uvas0d_wpftmp.AssemblyInfo.cs(15,59): warning CS7035: The specified version string does not conform to the recommended format - major.minor.build.revision [C:\Temp\wpf\test14\test14_10uvas0d_wpftmp.csproj]
obj\Debug\netcoreapp3.1\test14_10uvas0d_wpftmp.AssemblyInfo.cs(19,55): error CS8357: The specified version string contains wildcards, which are not compatible with determinism. Either remove wildcards from the version string, or disable determinism for this compilation [C:\Temp\wpf\test14\test14_10uvas0d_wpftmp.csproj]

The build failed. Fix the build errors and run again.

Finally, there is a error message that says that the problem is:

error CS8357: The specified version string contains wildcards, which are not compatible with determinism. Either remove wildcards from the version string, or disable determinism for this compilation

@vatsan-madhavan
Copy link
Member

/cc @SamBent - not a WPF bug.

@vatsan-madhavan vatsan-madhavan added Question General question, not a problem in source code or documentation (yet) and removed Bug Product bug (most likely) labels Feb 11, 2020
@the-black-wolf
Copy link
Author

@vatsan-madhavan I beg to differ, I think this is a bug in System.Windows.Navigation.BaseUriHelper.GetLoadedAssembly, attempting to determine loaded assembly using wrong version string, trying to instantiate it via Version object.

@the-black-wolf
Copy link
Author

Actually, by looking at the code, the bug is in the file https://github.com/dotnet/wpf/blob/master/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Navigation/BaseUriHelper.cs
in method GetAssemblyNameAndPart

@the-black-wolf
Copy link
Author

@vatsan-madhavan, @SamBent

Can you guys please confirm this as bug and relabel the issue?

@the-black-wolf
Copy link
Author

@vatsan-madhavan, @SamBent

Hmm, is there some process by which I can bring attention to this BUG. And it is a bug, and it is a regression. In .net framework 4.7.2/4.8 wpf this works just fine.

@rladuca
Copy link
Member

rladuca commented Feb 18, 2020

@vatsan-madhavan I think @the-black-wolf is correct that this is a WPF bug.

The documentation for System.Version assumes the following:

Version numbers consist of two to four components: major, minor, build, and revision. The major and minor components are required; the build and revision components are optional, but the build component is required if the revision component is defined. All defined components must be integers greater than or equal to 0. The format of the version number is as follows (optional components are shown in square brackets ([ and ]):

major.minor[.build[.revision]]

So it doesn't support wildcard versioning.

WPF is calling into Version.Parse with the wildcard URI (since it uses AssemblyFileVersion). That's not a valid version to parse.

I think either:

  • System.Version needs to support wildcard versions
  • WPF needs to somehow specially handle wildcard versions (or not use AssemblyFileVersion)

Thoughts?

@the-black-wolf I just created a .NET Framework project with Deterministic set to False and AssemblyVersion set to 1.0.* and got the exact same crash. Can you link a .NET Framework project that does not reproduce this issue?

@rladuca rladuca added Bug Product bug (most likely) and removed Question General question, not a problem in source code or documentation (yet) labels Feb 18, 2020
@rladuca
Copy link
Member

rladuca commented Feb 19, 2020

@SamBent @vatsan-madhavan Don't know what was going through my head earlier about versioning, ignore that.

@the-black-wolf I think I see the difference here. In .NET Framework you're likely using AssemblyVersionAttribute to set the wildcard and get the auto-increment. Is that the case?

I think the issue is that pathway doesn't really exist anymore. Using the AssemblyVersion property in the project (either in .NET Framework csproj or in new SDK-style csproj) results in both AssemblyVersionAttribute and AssemblyFileVersionAttribute both being set to the wildcard version. This is the cause of the issue you're seeing.

I think you might be able to turn off auto-generation of the assembly info via setting GenerateAssemblyInfo to false in the project file and then supply your own AssemblyVersionAttribute in order to use wildcards in the same way as in .NET Framework. I haven't tested that out just yet, but I'll try tomorrow.

@rladuca
Copy link
Member

rladuca commented Feb 19, 2020

Hmm, not sure this will work since PresentationBuildTasks is directly using AssemblyVersion to generate URIs (see here).

My understanding above was incorrect (again). What might be happening is that the AssemblyVersion isn't yet generated by the build by the time PBT gets around to creating the URI in .NET Core. I'll have to take a look at some binlogs to see if this is the case.

@weltkante
Copy link

weltkante commented Feb 19, 2020

Erm, wildcard versions are supposed to be resolved during build, not end up in any assembly attributes, the runtime should never see them. At least that was how it worked in Desktop.

I think the issue is that pathway doesn't really exist anymore.

If wildcard versions are not supported anymore by the build system it should raise a build error and not literally transcribe the placeholder into the version attribute. Wildcard versions at runtime are useless because they can only be resolved during a build.

Looking at dotnet/roslyn#22660 it sounds like wildards should be supported if deterministic build is off though

@rladuca
Copy link
Member

rladuca commented Feb 19, 2020

@weltkante Yes, I was getting a bit stream of consciousness above so probably a mess of half-conclusions.

My current thinking (have to validate with binlogs) is:

  1. In .NET Framework, you generally would put the wildcard in the AssemblyVersionAttribute and not in the AssemblyVersion property. Using the property there also breaks WPF in the exact same fashion.
  2. This says to me that there is a step in .NET Framework that converts AssemblyVersionAttribute to the AssemblyVersion property and that this step occurs prior to PresentationBuildTasks using AssemblyVersion.
  3. In .NET Core, this doesn't seem to happen. AssemblyVersion remains empty even when overriding the default generated AssemblyInfo.cs. While the wildcard is filled in when the assembly is built, PresentationBuildTasks sees an empty string. This fixes the originally reported issue, but breaks my workaround since the resource URI is no longer valid to load the resource.
  4. In either .NET Framework or .NET Core, if you set the AssemblyVersion property, PresentationBuildTasks sees not the filled in version, but the original wildcard string. Meaning it was always too early in the build process when using the property.

Again, this is just from experimentation right now, I have to comb through logs to see what is happening when (unless someone knows offhand).

Hopefully that condenses and clarifies the word salad I was typing last night : - )

@rladuca
Copy link
Member

rladuca commented Feb 19, 2020

So I went out and did some testing and I actually haven't been able to get this to work on .NET Framework.

If you simply take a new .NET Framework WPF project, set Deterministic to False and update the AssemblyVersionAttribute to a wildcard everything seems fine. After you add a page and reference it as a source for a ResourceDictionary, you get the same issue as I was seeing in the attempted workaround for .NET Core. Basically, the AssemblyVersion consumed by MarkupCompilePass1 is empty, creating bad resource URIs which then break the app at runtime.

@the-black-wolf When you were saying this would break a forward port of a .NET Framework app, is this the method you were using? If not, can you tell me the method you are using?

@the-black-wolf
Copy link
Author

Just to confirm you comment, yes the framework version that works uses AssemblyVersionAttribute in , as was dictated by old project format.

Also, setting GenerateAssemblyInfo to false and supplying attribute in .net core works in my sample app, the dictionary is loaded and version is autoincremented. (see below). I will have to check against the main projects list tomorrow. If it works, I think it is a viable workaround until you guys fix this in 5.0.

image

@the-black-wolf
Copy link
Author

the-black-wolf commented Feb 19, 2020

@the-black-wolf When you were saying this would break a forward port of a .NET Framework app, is this the method you were using? If not, can you tell me the method you are using?

Hmm, that seems to work for me. Here's my solution with two apps, one core one frame, both created that way. Maybe I did something wrong, I played around a bit with this.

WpfApp-nondeterm.zip

@weltkante
Copy link

weltkante commented Feb 19, 2020

I think thats a VS/msbuild issue not specific to WPF, setting <AssemblyVersion>1.0.*</AssemblyVersion> in a new-style project generates both AssemblyVersion as well as AssemblyFileVersion and the latter doesn't understand autoincremented placeholders. If you actually specify the <FileVersion> as well everything works as it used to in Desktop Framework for me.

Its particular bad because when you configure the versions from the UI (in contrasts to editing the project file) you cannot see the difference between <FileVersion> specified and having its implicit default value.

You set Assembly Version from project property UI to include a placeholder. In old-style projects this edits the AssemblyInfo.cs and doesn't touch AssemblyFileVersion. In new style projects this will leave FileVersion unspecified and generate both AssemblyVersion and AssemblyFileVersion to include the placeholder at build even though the UI shows Assembly file version as having no placeholder. The project property UI is not representing what gets generated.

Specifying both

  <PropertyGroup>
    <AssemblyVersion>1.0.0.*</AssemblyVersion>
    <FileVersion>1.0.0.0</FileVersion>
  </PropertyGroup>

in a new style project generates the correct attributes even though the UI just looks like as if FileVersion wasn't specified.

@rladuca
Copy link
Member

rladuca commented Feb 19, 2020

@the-black-wolf Got it, my issue was I was using a full page in the ResourceDictionary reproduction (thanks to @rrelyea setting me straight there). That's why my workaround wasn't working and I was having issues in Framework as well.

I am not sure there is anything to fix in WPF specifically. You can get more specific if you'd like and instead of setting GenerateAssemblyInfo to false, use GenerateAssemblyVersionAttribute. That will allow you to specify just the AssemblyVersionAttribute and keep the rest around without generating conflicts.

@rladuca
Copy link
Member

rladuca commented Feb 19, 2020

@weltkante Have you tried adding a new Page and then instantiating it? MarkupCompilePass1 always gets the wildcard string and you're going to run into the Version.Parse issue originally reported. I think you can't set the property specifically.

@the-black-wolf
Copy link
Author

Well, I would think its fixable, no? Ultimately, wildcard or not in build, the dictionary is loaded on startup, no? At that point you have a running assembly from which to source it and a way to determine its version.
I am trying to make head and tails of your comments, honestly :) it seem to me that this whole versioning thing in .net core hasn't been thought out properly in the rush to discard Assemblyinfo.cs.
I am not married to that file, honestly, I think it was a good idea to be rid of it in favor of a project file, but that comes with a certain obligation to have that stuff work properly. I think FileVersion is a red herring here, it shouldnt really mater what its set to. The only relevant info should be AssemblyVersion, and not the version specified in source, but the version created from it in build.

@weltkante
Copy link

weltkante commented Feb 19, 2020

@rladuca ok instantiating a page breaks new-style WPF projects even if they specify <FileVersion> in the project file to contain no placeholders. It works in old style .NET WPF projects though where things are specified in AssemblyInfo.cs

@rladuca
Copy link
Member

rladuca commented Feb 19, 2020

@the-black-wolf Whether or not we can do something so that MarkupCompilePass1 can get access to the realized AssemblyVersion when it is set via the property is something I cannot answer. WPF has to rely on the property being set to the real version at the correct point in the build. If you set the AssemblyVersion property in a .NET Framework project this will still break.

@weltkante Correct, as long as you don't use set the versions via properties it works. Using a property breaks the generated code for InitializeComponent in MarkupCompilePass1 (and probably some other places we just haven't found).

I think the most reasonable answer is that, in .NET Framework, you altered the AssemblyInfo (either directly or via UI) to get this behavior and you should continue to do so in .NET Core. That makes the most sense from a porting perspective and also works with minimal effort.

@rladuca
Copy link
Member

rladuca commented Feb 19, 2020

@the-black-wolf I sort of see what you're saying. We could, potentially use GetExecutingAssembly to fill in the URI on the fly instead of spitting out a string in the generation of InitializeComponent. I looked through PresentationBuildTasks and I don't really see the version being used apart from that generation step (and as an input into the compiler state).

I don't have a good answer for any compat concerns that may cause. @SamBent @vatsan-madhavan do you have any opinions on potentially changing the generated code for InitializeComponent to query the executing assembly version instead of using a string?

@weltkante
Copy link

weltkante commented Feb 19, 2020

You might need to annotate InitializeComponent with [MethodImpl(MethodImplOptions .NoInlining)] to guarantee that GetExecutingAssembly picks up the right assembly - otherwise a particular short InitializeComponent within a particular short constructor might get inlined and report the wrong assembly (I don't know if constructor inlining is a thing but if you rely on GetExecutingAssembly you'll want inlining disabled to future-proof yourself against JIT changes)

An alternative option not relying on GetExecutingAssembly is emitting typeof(xxx).Assembly where you fill in the type you are generating the code for. (Don't use GetType() on the current instance though since that will break when you're subclassing a Page/UserControl, non-WPF XAML sometimes had this problem, I forgot if it was Win8 or UWP.)

@rladuca
Copy link
Member

rladuca commented Feb 19, 2020

Here is a summary of what we know and what has been discussed.

Issue

  • MarkupCompilePass1 consumes the AssemblyVersion build property and uses it to generate the resource URI in the code for InitializeComponent. This code is generated any time BAML is needed.
    if (IsBamlNeeded)
    {
    GenerateInitializeComponent(IsCompilingEntryPointClass);
    • When this is a wildcard version string (e.g. the developer has defined it as such via the AssemblyVersion property), this will cause a crash at runtime. In the call tree from InitializeComponent, WPF is calling Version.Parse with the wildcard string. Since this is not a valid version string, an exception is thrown.
    • This is not a regression, the same behavior exists in .NET Framework. It is, however, worse to have this behavior in .NET Core as there is a slightly different expectation on how to accomplish wildcard versions as compared to .NET Framework.

Workaround

  • As of right now, if you need to use wildcard versions in .NET Core, you must set GenerateAssemblyVersionAttribute to False in your project and then add the wildcard as an AssemblyVersionAttribute to your AssemblyInfo.cs. (Example Here)
    • When these steps are followed the AssemblyVersion property passed to WPF is an empty string and the generated code for InitializeComponent does not have a version in the resource URI.
    • This is essentially the same process and result (sans turning off generation of AssemblyVersionAttribute) as .NET Framework.

Potential Fix

  • After discussions with @vatsan-madhavan , we decided that a fix can be made for the issue by simply detecting the wildcard version string in GenerateInitializeComponent and ensuring the version string used in the resource URI is empty if a wildcard is in use.
    • This mirrors the state of the URI in both the workaround and the way that you would have to do this in .NET Framework.
    • Uses of WPF with non-wildcard version strings (either via the AssemblyVersion build property or the AssemblyVersionAttribute in AssemblyInfo.cs) are not affected.
    • I have a WIP branch here: https://github.com/rladuca/wpf/tree/pbtwildcards. I have tested this with some basic applications and all is as expected.

@the-black-wolf
Copy link
Author

Thanks. The workaround works ok on our projects, so we'll use that in anticipation of the official fix.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug Product bug (most likely)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants