Make StronglyTypedResourceGenerator work on .NET Core #3819

wjk · 2018-10-02T23:34:35Z

Title says it all, really. Have tested with the project attached below. The STR code-behind is generated containing both a string and non-string resource (a Bitmap, to be precise) and compiles successfully. Code is very messy, however; I would appreciate some advice on how it should be cleaned up. Thanks!

Relates to: #2221, dotnet/sdk#1782

msbuild_test.zip

Methods that used WinForms-specific resx BCL types have been #if'ed out to throw NotImplementedException on .NET Core. They will be more fully implemented in the next series of commits.

This helps work with non-string types in resx files.

wjk · 2018-10-02T23:36:38Z

Note that ResXFileRef resource entries won't be handled properly with the code in this PR. The generated property will reference the type ResXFileRef, not Stream, Bitmap, or whatever other type is correct in this case. A pointer on how to fix this will also be appreciated.

wjk · 2018-10-03T00:08:04Z

I think I've fixed ResXFileRef support, but I'm not certain.

rainersigwald · 2018-10-04T22:33:04Z

@nguerrera and @Tanya-Solyanik, do you have thoughts about this approach of reimplementing some of the resx reading instead of (I assume until) we can wait to get System.Windows.Forms and a full ResXResourceReader? It seems like a fine approach to me.

nguerrera · 2018-10-04T22:41:34Z

src/Tasks/system.design/stronglytypedresourcebuilder.cs

-            while (!type.IsPublic)
-            {
-                type = type.BaseType;
-            }


It's not clear to me why this can be deleted. Won't it break existing uses?

It had to be deleted because I'm no longer operating on types. I am dealing with the full names of types as strings, because MSBuild might not have access to the types (such as Bitmap) that the user is placing in their resx file.

I see that. I think we would have to leave the prior behavior on full-framework msbuild with more conditional compilation. :(

yeah, I'm afraid so :(

@dreddy-work

Just my 2 cents, but I feel it would be really useful to have implementations of System.Resources.IResourceReader and System.Resources.IResourceWriter available to MSBuild and cross-platform (hopefully wrapped as a .netstandard package so it can fall through to the full framework or use a custom implementation etc.)
I believe that EF6 will need this as part of its porting.

nguerrera · 2018-10-04T22:43:43Z

@rainersigwald I don't have a strong opinion. If the amount of code required is small (seems so here), I think it's reasonable to duplicate. Otherwise, I think we'd want to push the resx stuff down from System.Windows.Forms to base shared framework somewhere.

wjk · 2018-10-04T22:55:14Z

@nguerrera @rainersigwald In all honesty, this is still a very, very rough first draft. Any and all advice on how to improve this PR will be appreciated.

Per PR review. I have tested this locally, and it works, but I don't know it if meets compatibility requirements or not.

I had to add a reference to the ConfigurationManager package so that the ConfigurationException thrown by the CodeDomProvider can be properly handled. This in turn required that I bump the version of the System.Security.Principal.Windows package.

iSazonov

I'm happy to see the PR. In PowerShell repo we use a workaround some years. I hope the PR will successfully finished.
I don't see xUnit tests so I'd consider shared code with WinForm too.

src/Tasks/GenerateResource.cs

src/Tasks/system.design/stronglytypedresourcebuilder.cs

src/Tasks/GenerateResource.cs

src/Tasks/system.design/stronglytypedresourcebuilder.cs

Co-Authored-By: wjk <wjk011@gmail.com>

Sometimes (such as in the test resx file I'm using), the strings used in the resx file can be very long. Note that I don't know how much this fix changes the doc comment generation behavior from what it was before (if, indeed, it changed at all).

wjk · 2018-11-13T18:54:25Z

I think I've figured out the failing unit test. The problem is that the value returned by the IResourceReader here is null. If the value is null, then there is by definition no way to get its type. Unfortunately, as I understand it this resource should not be null in the first place. I don't know what's going on here.

@rainersigwald @nguerrera Could you (or whomever else has an idea) please chime in with some thoughts? I don't see a way to fix this test, but I may be missing something obvious. Thanks!

src/Tasks/GenerateResource.cs

wjk · 2018-11-15T16:00:47Z

Could someone please help me out with the failing CI? I keep getting ~70 unit test failures, even after reverting the change I made in e45c285. All of these tests pass when I run them locally on my dev box. I am at my wits' end here, and I want to get this PR merged sooner rather than later so it can be ready for the Core 3.0 official preview.

rainersigwald · 2018-11-15T16:13:47Z

I can reproduce some of the failures with build.cmd -test on my machine:

On full framework,

 Microsoft.Build.UnitTests.GenerateResource_Tests.InProc.References.ReferencedAssemblySpecifiedUsingRelativePath
System.NullReferenceException : Object reference not set to an instance of an object.
   at Microsoft.Build.Tasks.ProcessResourceFiles.AddResource(ReaderInfo reader, String name, Object value, String inputFileName, Int32 lineNumber, Int32 linePosition, String customTypeName) in /_/src/Tasks/GenerateResource.cs:line 3774
   at Microsoft.Build.Tasks.ProcessResourceFiles.AddResource(ReaderInfo reader, String name, Object value, String inputFileName, String customTypeName) in /_/src/Tasks/GenerateResource.cs:line 3786
   at Microsoft.Build.Tasks.ProcessResourceFiles.ReadResources(ReaderInfo readerInfo, IResourceReader reader, String fileName) in /_/src/Tasks/GenerateResource.cs:line 3493
   at Microsoft.Build.Tasks.ProcessResourceFiles.ReadResources(String filename, Boolean shouldUseSourcePath, String outFileOrDir) in /_/src/Tasks/GenerateResource.cs:line 3044
   at Microsoft.Build.Tasks.ProcessResourceFiles.ProcessFile(String inFile, String outFileOrDir) in /_/src/Tasks/GenerateResource.cs:line 2625
   at Microsoft.Build.Tasks.ProcessResourceFiles.Run(TaskLoggingHelper log, ITaskItem[] assemblyFilesList, List`1 inputs, List`1 satelliteInputs, List`1 outputs, Boolean sourcePath, String language, String namespacename, String resourcesNamespace, String filename, String classname, Boolean publicClass, Boolean extractingResWFiles, String resWOutputDirectory) in /_/src/Tasks/GenerateResource.cs:line 2481
   at Microsoft.Build.Tasks.ProcessResourceFiles.Run(TaskLoggingHelper log, ITaskItem[] assemblyFilesList, List`1 inputs, List`1 satelliteInputs, List`1 outputs, Boolean sourcePath, String language, String namespacename, String resourcesNamespace, String filename, String classname, Boolean publicClass, Boolean extractingResWFiles, String resWOutputDirectory)
   at Microsoft.Build.Tasks.GenerateResource.Execute() in /_/src/Tasks/GenerateResource.cs:line 816
   at Microsoft.Build.UnitTests.GenerateResource_Tests.InProc.References.ReferencedAssemblySpecifiedUsingRelativePath() in S:\msbuild\src\Tasks.UnitTests\GenerateResource_Tests.cs:line 2837
Output:
resgen.exe /r:bin\debug\ClassLibrary20.dll /compile C:\Users\raines\AppData\Local\Temp\nci4bpf2.qsd\1ee678c1eead440488532fd43f334547\MyStrings.resx,C:\Users\raines\AppData\Local\Temp\nci4bpf2.qsd\1ee678c1eead440488532fd43f334547\MyStrings.resources
Creating a separate AppDomain because of resource "Image1" of type "ClassLibrary20.Class1, ClassLibrary20, version=1.0.0.0, Culture=neutral, PublicKeyToken=null" in "C:\Users\raines\AppData\Local\Temp\nci4bpf2.qsd\1ee678c1eead440488532fd43f334547\MyStrings.resx" on line 43.
Processing resource file "C:\Users\raines\AppData\Local\Temp\nci4bpf2.qsd\1ee678c1eead440488532fd43f334547\MyStrings.resx" into "C:\Users\raines\AppData\Local\Temp\nci4bpf2.qsd\1ee678c1eead440488532fd43f334547\MyStrings.resources".

On core:

      <test name="Microsoft.Build.UnitTests.WriteCodeFragment_Tests.InvalidLanguage" type="Microsoft.Build.UnitTests.WriteCodeFragment_Tests" method="InvalidLanguage" time="0.0014599" result="Fail">
        <traits>
          <trait name="Category" value="mono-osx-failing" />
        </traits>
        <failure exception-type="System.CodeDom.Compiler.CodeDomProvider+ConfigurationErrorsException">
          <message><![CDATA[System.CodeDom.Compiler.CodeDomProvider+ConfigurationErrorsException : There is no CodeDom provider defined for the language.]]></message>
          <stack-trace><![CDATA[   at System.CodeDom.Compiler.CodeDomProvider.GetCompilerInfo(String language)
   at System.CodeDom.Compiler.CodeDomProvider.CreateProvider(String language)
   at Microsoft.Build.Tasks.WriteCodeFragment.GenerateCode(String& extension) in /_/src/Tasks/WriteCodeFragment.cs:line 140
   at Microsoft.Build.Tasks.WriteCodeFragment.Execute() in /_/src/Tasks/WriteCodeFragment.cs:line 84
   at Microsoft.Build.UnitTests.WriteCodeFragment_Tests.InvalidLanguage() in S:\msbuild\src\Tasks.UnitTests\WriteCodeFragment_Tests.cs:line 34]]></stack-trace>
        </failure>
      </test>

      <test name="Microsoft.Build.UnitTests.GenerateResource_Tests.InProc.RequiredTransformations.STRWithResourcesNamespaceVB" type="Microsoft.Build.UnitTests.GenerateResource_Tests.InProc.RequiredTransformations" method="STRWithResourcesNamespaceVB" time="0.0882194" result="Fail">
        <output><![CDATA[resgen.exe C:\Users\raines\AppData\Local\Temp\ogfuf5ii.wzw\tmpbedb2c14b77e4be3b605675e84a9533a.txt C:\Users\raines\AppData\Local\Temp\ogfuf5ii.wzw\tmpbedb2c14b77e4be3b605675e84a9533a.resources /str:VB,,,
Processing resource file "C:\Users\raines\AppData\Local\Temp\ogfuf5ii.wzw\tmpbedb2c14b77e4be3b605675e84a9533a.txt" into "C:\Users\raines\AppData\Local\Temp\ogfuf5ii.wzw\tmpbedb2c14b77e4be3b605675e84a9533a.resources".
Processing 4 resources from file "C:\Users\raines\AppData\Local\Temp\ogfuf5ii.wzw\tmpbedb2c14b77e4be3b605675e84a9533a.txt".
Creating strongly typed resources class "C:\Users\raines\AppData\Local\Temp\ogfuf5ii.wzw\tmpbedb2c14b77e4be3b605675e84a9533a.vb".
]]></output>
        <traits>
          <trait name="Category" value="mono-osx-failing" />
          <trait name="Category" value="mono-windows-failing" />
        </traits>
        <failure exception-type="Xunit.Sdk.TrueException">
          <message><![CDATA[Assert.True() Failure\r\nExpected: True\r\nActual:   False]]></message>
          <stack-trace><![CDATA[   at Microsoft.Build.UnitTests.GenerateResource_Tests.Utilities.AssertStateFileWasWritten(GenerateResource t) in S:\msbuild\src\Tasks.UnitTests\GenerateResource_Tests.cs:line 3245
   at Microsoft.Build.UnitTests.GenerateResource_Tests.Utilities.STRNamespaceTestHelper(String strLanguage, String resourcesNamespace, String classNamespace, ITestOutputHelper output) in S:\msbuild\src\Tasks.UnitTests\GenerateResource_Tests.cs:line 3631
   at Microsoft.Build.UnitTests.GenerateResource_Tests.InProc.RequiredTransformations.STRWithResourcesNamespaceVB() in S:\msbuild\src\Tasks.UnitTests\GenerateResource_Tests.cs:line 1258]]></stack-trace>
        </failure>
      </test>

      <test name="Microsoft.Build.UnitTests.GenerateResource_Tests.InProc.RequiredTransformations.STRWithResourcesNamespaceAndSTRNamespaceCS" type="Microsoft.Build.UnitTests.GenerateResource_Tests.InProc.RequiredTransformations" method="STRWithResourcesNamespaceAndSTRNamespaceCS" time="0.0226973" result="Fail">
        <output><![CDATA[resgen.exe C:\Users\raines\AppData\Local\Temp\ogfuf5ii.wzw\tmp0dc3042b713248e1b7681137c6c57fa7.txt C:\Users\raines\AppData\Local\Temp\ogfuf5ii.wzw\tmp0dc3042b713248e1b7681137c6c57fa7.resources /str:CSharp,MySTClassNamespace,,
Processing resource file "C:\Users\raines\AppData\Local\Temp\ogfuf5ii.wzw\tmp0dc3042b713248e1b7681137c6c57fa7.txt" into "C:\Users\raines\AppData\Local\Temp\ogfuf5ii.wzw\tmp0dc3042b713248e1b7681137c6c57fa7.resources".
Processing 4 resources from file "C:\Users\raines\AppData\Local\Temp\ogfuf5ii.wzw\tmp0dc3042b713248e1b7681137c6c57fa7.txt".
Creating strongly typed resources class "C:\Users\raines\AppData\Local\Temp\ogfuf5ii.wzw\tmp0dc3042b713248e1b7681137c6c57fa7.cs".
]]></output>
        <traits>
          <trait name="Category" value="mono-osx-failing" />
          <trait name="Category" value="mono-windows-failing" />
        </traits>
        <failure exception-type="Xunit.Sdk.TrueException">
          <message><![CDATA[Assert.True() Failure\r\nExpected: True\r\nActual:   False]]></message>
          <stack-trace><![CDATA[   at Microsoft.Build.UnitTests.GenerateResource_Tests.Utilities.AssertStateFileWasWritten(GenerateResource t) in S:\msbuild\src\Tasks.UnitTests\GenerateResource_Tests.cs:line 3245
   at Microsoft.Build.UnitTests.GenerateResource_Tests.Utilities.STRNamespaceTestHelper(String strLanguage, String resourcesNamespace, String classNamespace, ITestOutputHelper output) in S:\msbuild\src\Tasks.UnitTests\GenerateResource_Tests.cs:line 3631
   at Microsoft.Build.UnitTests.GenerateResource_Tests.InProc.RequiredTransformations.STRWithResourcesNamespaceAndSTRNamespaceCS() in S:\msbuild\src\Tasks.UnitTests\GenerateResource_Tests.cs:line 1271]]></stack-trace>
        </failure>
      </test>

And others. How are you running the tests?

wjk · 2018-11-15T16:20:36Z

@rainersigwald I just run them using the VS Test Explorer. I did this so I could use the debugger to investigate the cause of the failures. Is this an incorrect methodology?

Apparently, this was not the cause of the excessive test failures being seen in CI. This reverts commit e45c285.

wjk · 2018-11-15T16:50:04Z

As it turns out, using the VS Test Explorer was incorrect methodology. It runs the tests under the full-framework runtime, while the tests were failing only when run under the .NET Core runtime. Will have some fixes pushed up momentarily.

This works around unit test failures on macOS and Linux that I do not have the patience to fix correctly.

wjk · 2018-12-02T18:52:31Z

@rainersigwald @nguerrera It's been over two weeks since I got the tests passing; I would appreciate PR review so we can get this merged soon. Thanks! 😄

nguerrera · 2018-12-07T20:52:20Z

src/Tasks/GenerateResource.cs

            {
                provider = CodeDomProvider.CreateProvider(stronglyTypedLanguage);
            }
-#if FEATURE_SYSTEM_CONFIGURATION


Elsewhere you still have configuration #if'ed out. Does CodeDomProvider actually throw configuration exception on .NET Core? I think we should avoid adding a dependency on it if it's not needed.

nguerrera · 2018-12-07T20:53:00Z

src/Tasks/WriteCodeFragment.cs

            {
                provider = CodeDomProvider.CreateProvider(Language);
            }
+#if FEATURE_WINFORMS_RESX


(This is where it's still #if'ed out that I referenced in earlier comment)

nguerrera · 2018-12-07T20:56:06Z

src/Tasks/system.design/stronglytypedresourcebuilder.cs


            return InternalCreate(resourceList, baseName, generatedCodeNamespace, resourcesNamespace, codeProvider, internalClass, out unmatchable);
+#else
+            throw new NotImplementedException();


This should probably have a better error message describing what isn't supported.

I don't know about MSBuild usage, but in CoreFX NotImplementedException is used when something is not implemented by design (such as, it needs to be overridden) and PlatformNotSupportedException when it's not relevant to the platform or not yet implemented there.

nguerrera · 2018-12-07T21:05:41Z

In general, I still find it hard to follow the impact this will have on the full-framework case. Some work was done to retain the System.Type usage, but then in some places, there are still string comparison replacing type comparisons unconditionally in other places. I think I'd have an easier time reviewing if it were more obvious that this enables core without changing behavior on framework.

wjk · 2018-12-07T22:32:48Z

@nguerrera In all honesty, the string comparison was only implemented because, at the time, ResXResourceReader and friends were not available on Core. If there was any other way, I would have used it.

However, with WinForms open-sourced and ported to Core, I was planning on adding a copy of them into the Tasks project (to break the dependency on the Forms DLL; we can always remove them again later), getting rid of the string-related code, and enabling FEATURE_RESX_RESOURCE_READER globally. Sound good? Thanks!

nguerrera · 2018-12-07T22:36:30Z

I'm not sure because there are open questions about actively instantiating objects while reading resx. I have two things in mind: get away from that, but also don't ~~change~~break the .net framework case. I'd have to look more closely at what ResxResourceReader does.

rainersigwald · 2018-12-07T22:37:32Z

I'd have to look more closely at what ResxResourceReader does.

This is high on my agenda for next week, for reasons closely related to this PR.

nguerrera · 2018-12-07T22:41:40Z

This is high on my agenda for next week, for reasons closely related to this PR.

👍

@wjk, I would hold off on doing any big changes to this PR while @rainersigwald is looking into this.

wjk · 2019-02-03T02:11:33Z

@nguerrera @rainersigwald Any updates?

bergmeister · 2019-02-18T07:03:31Z

This feature would really be useful, thanks to the PR creator, I hope this gets merged downstream to the 2.2 SDK as well.

bergmeister · 2019-03-21T06:41:32Z

Any updates @nguerrera @rainersigwald ?

wjk · 2019-06-26T19:41:02Z

This PR has since been superseded by #4420. Closing.

radical and others added 7 commits December 19, 2016 10:18

Enable Mono CI builds (dotnet#1481)

02f1e31

Revert "Enable Mono CI builds"

9f13df6

Merge branch 'master' of github.com:Microsoft/msbuild

d7bf820

First step at porting StronglyTypedResourceBuilder

90d20bf

Methods that used WinForms-specific resx BCL types have been #if'ed out to throw NotImplementedException on .NET Core. They will be more fully implemented in the next series of commits.

Fix build under FEATURE_CODEDOM && .NET Core

54dc675

Update StronglyTypedResourceBuilder to operate on type name strings

3f80285

This helps work with non-string types in resx files.

Fix non-string resource handling

acdea2e

wjk added 2 commits October 2, 2018 19:37

Fix indentation error

f45af2a

Fix handling of ResXFileRef objects

6a3e243

nguerrera reviewed Oct 4, 2018

View reviewed changes

rainersigwald mentioned this pull request Oct 5, 2018

Avoid casting Enumerable.Empty to IList #3836

Merged

wjk added 2 commits October 5, 2018 17:00

Update STRB to work on types as well as type names

0242f82

Per PR review. I have tested this locally, and it works, but I don't know it if meets compatibility requirements or not.

Fix failing unit test

3660242

I had to add a reference to the ConfigurationManager package so that the ConfigurationException thrown by the CodeDomProvider can be properly handled. This in turn required that I bump the version of the System.Security.Principal.Windows package.

wjk mentioned this pull request Oct 17, 2018

MSBuild generators aren't picked up when using item update mono/monodevelop#6372

Open

rainersigwald mentioned this pull request Nov 12, 2018

Generating strongly typed resource files not currently supported on .NET Core MSBuild #2272

Closed

iSazonov reviewed Nov 13, 2018

View reviewed changes

iSazonov and others added 5 commits November 13, 2018 09:59

Use StringComparison.Ordinal

9348e5b

Co-Authored-By: wjk <wjk011@gmail.com>

Use StringComparison.Ordinal

be56e2c

Co-Authored-By: wjk <wjk011@gmail.com>

Fix build

8f448bc

Merge branch 'master' into resgen-improvements

b60a54a

iSazonov reviewed Nov 14, 2018

View reviewed changes

src/Tasks/GenerateResource.cs Outdated Show resolved Hide resolved

wjk added 2 commits November 14, 2018 11:34

Address PR feedback

6dc39a1

Address more PR feedback

b118be6

Reinstate "Use typeof(object) when the value is null"

7e54b77

Apparently, this was not the cause of the excessive test failures being seen in CI. This reverts commit e45c285.

wjk and others added 5 commits November 15, 2018 11:57

Fix failing unit tests

5414146

Fix test failure

1143d53

Enable FEATURE_CODEDOM only for Tasks and its unit tests

019bff0

This works around unit test failures on macOS and Linux that I do not have the patience to fix correctly.

Catch appropriate exception for .NET Core CodeDom

06e1bc2

Fix unit test on non-Windows platform

57d9011

wjk mentioned this pull request Dec 5, 2018

InvalidCastException when icon is set in autogenerated form designer code dotnet/winforms#163

Closed

nguerrera reviewed Dec 7, 2018

View reviewed changes

nguerrera mentioned this pull request Dec 15, 2018

Build time ResX code-behind generation dotnet/sdk#1782

Closed

rainersigwald added this to the .NET Core 3.0 milestone Mar 21, 2019

wjk closed this Jun 26, 2019

rainersigwald mentioned this pull request Jul 24, 2019

WriteCodeFragment fails in .NET Core MSBuild if input has braces #937

Closed

rainersigwald modified the milestones: .NET Core 3.0, MSBuild 16.3 Jul 26, 2019

Uh oh!

Make StronglyTypedResourceGenerator work on .NET Core #3819

Make StronglyTypedResourceGenerator work on .NET Core #3819

Uh oh!

Conversation

wjk commented Oct 2, 2018

Uh oh!

wjk commented Oct 2, 2018

Uh oh!

wjk commented Oct 3, 2018

Uh oh!

rainersigwald commented Oct 4, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nguerrera commented Oct 4, 2018

Uh oh!

wjk commented Oct 4, 2018

Uh oh!

iSazonov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wjk commented Nov 13, 2018

Uh oh!

Uh oh!

wjk commented Nov 15, 2018

Uh oh!

rainersigwald commented Nov 15, 2018

Uh oh!

wjk commented Nov 15, 2018

Uh oh!

wjk commented Nov 15, 2018

Uh oh!

wjk commented Dec 2, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nguerrera commented Dec 7, 2018

Uh oh!

wjk commented Dec 7, 2018

Uh oh!

nguerrera commented Dec 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rainersigwald commented Dec 7, 2018

Uh oh!

nguerrera commented Dec 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wjk commented Feb 3, 2019

Uh oh!

bergmeister commented Feb 18, 2019

Uh oh!

bergmeister commented Mar 21, 2019

Uh oh!

wjk commented Jun 26, 2019

Uh oh!

nguerrera commented Dec 7, 2018 •

edited

Loading

nguerrera commented Dec 7, 2018 •

edited

Loading