Skip to content

Roslyn-based code task factory#3175

Merged
AndyGerlicher merged 5 commits intodotnet:masterfrom
jeffkl:codetaskfactory
May 13, 2018
Merged

Roslyn-based code task factory#3175
AndyGerlicher merged 5 commits intodotnet:masterfrom
jeffkl:codetaskfactory

Conversation

@jeffkl
Copy link
Copy Markdown
Contributor

@jeffkl jeffkl commented Apr 9, 2018

Compiles an in-memory netstandard2.0 assembly using csc.exe and loads the task. This means it uses Roslyn compiler instead of CodeDom so it can be cross platform.

I've implemented it to follow the same task XML as the existing CodeTaskFactory so that existing in-line tasks can move to it by changing the task factory name.

<UsingTask  
    TaskName="DoNothing"  
    TaskFactory="RoslynCodeTaskFactory"  
    AssemblyFile="$(MSBuildToolsPath)\Microsoft.Build.Tasks.Core.dll" >  
    <ParameterGroup />  
    <Task>  
      <Reference Include="" />  
      <Using Namespace="" />  
      <Code Type="Fragment" Language="cs">  
      </Code>  
    </Task>  
  </UsingTask>  

There is concern that there might be a breaking change some where so I can't replace the existing CodeTaskFactory. Users will need to opt-in to it.

I also need to update documentation so that its discoverable and deprecate my Roslyn-based code task factory.

Follow-up items:

@jeffkl jeffkl force-pushed the codetaskfactory branch 6 times, most recently from 775577b to f97d349 Compare April 27, 2018 21:14
@jeffkl jeffkl changed the title WIP: Roslyn-based code task factory Roslyn-based code task factory Apr 30, 2018
@jeffkl
Copy link
Copy Markdown
Contributor Author

jeffkl commented Apr 30, 2018

@jaredpar FYI I'm finally going to ship this as part of MSBuild since we're on NETStandard2.0. We still have to ship ref assemblies netstandard.dll and mscorlib.dll but that's a lot less than before.

@jeffkl jeffkl force-pushed the codetaskfactory branch from 817855d to fe76828 Compare May 3, 2018 22:44
jeffkl added 2 commits May 8, 2018 08:43
… assembly by generating source code and executing the managed compiler
@jeffkl jeffkl force-pushed the codetaskfactory branch from fe76828 to d2e9a67 Compare May 8, 2018 15:44
@AndyGerlicher
Copy link
Copy Markdown
Contributor

Pushed a merge commit to resolve a conflict from DownloadFile

@AndyGerlicher AndyGerlicher merged commit aafd6ed into dotnet:master May 13, 2018
@jeffkl jeffkl deleted the codetaskfactory branch May 14, 2018 13:41
josephmyers added a commit to josephmyers/liblcm that referenced this pull request Jun 6, 2022
The latter is cross-platform, where the former isn't. See dotnet/msbuild#3175 for more information. Note that this functionality requires VS 15.8 or later. This change allows "dotnet build -c Release" to pass from cmd (it is a little flaky, just like the IDE build, but re-running enough times will fix it).
josephmyers added a commit to sillsdev/liblcm that referenced this pull request Jun 9, 2022
* Improve exception message for duplicate styles (#195)

* Added NUnit3TestAdapter as a package reference to the four test projects. This was necessary in VS2022, since there is currently no NUnit extension.

* Updating SIL.LCModel.Utils to .NET Standard 2.0

This will allow it to target frameworks other than .NET Framework, like .NET Core. This is the first and most isolated project in this solution -- the others will follow.

* Updating SIL.LCModel.Build.Tasks to .NET Standard 2.0

This will allow it to target frameworks other than .NET Framework, like .NET Core. This required a new version of SIL.IdlImporter, and it now compiles with a warning (from IdlImporter) regarding an inner reference to a custom AntLr package that still targets 4.6.1. At this point, it's just a hope that everything stills works.

* Updating Tools project from Framework 4.6.1 to Standard 2.0.

* Updating SIL.LCModel.Core to .NET Standard 2.1

This will allow it to target frameworks other than .NET Framework, like .NET Core. This is contrast to the other projects at Standard 2.0, as this one required a few attributes for the auto-generated Kernel.cs file. I'm not sure yet what this means for the dependencies still on Framework (there are a slew of build warnings).

* Updating SIL.LCModel to .NET Standard 2.1

This will allow it to target frameworks other than .NET Framework, like .NET Core. This is a dual target with .NET Framework now. Because of the inner dependency by LCModel.Core on 2.1, all projects that reference it must use the same dual target. This compiles with multiple warnings, and it's hard to tell if any need to be heeded.

* Updating SIL.LCModel.FixData to .NET Standard 2.1

This will allow it to target frameworks other than .NET Framework, like .NET Core. This is a dual target with .NET Framework now. Because of the inner dependency by LCModel.Core on 2.1, all projects that reference it must use the same dual target. This compiles with many warnings (for Framework dependencies) as they build up through the various referenced projects.

* Re-enabling PrivateAssets property for IdlImporter in Build.Tasks

In trying to determine why IdlImporter and other references are no longer being copied to the artifacts folder, I noticed this flag was removed during the .NET Standard upgrade. Restoring it here, as it's one less difference between old and new.

* Added CopyLocalLockFileAssemblies to Build.Tasks.csproj.

This is necessary with .NET Standard now to copy over dependent assemblies (like IDLImporter.dll) to the output folder, as Framework did.

* Added NHunspell.Patched reference back to .Core.csproj.

Oddly enough, had to keep the NHunspell reference, as well. With the .Patched only, the C# symbols were never able to be resolved, but the .Patched is still necessary to copy/provide the two x64/x86 DLL's to the output folder.

* Getting closer to building .Core successfully in .NET Standard 2.0

This involved: Upgrading Build.Tasks to the latest version of IdlImporter, hopefully for the last time. Re-adding Icu4c*.Lib package back, as it was pre-upgrade. Fixing OutDir property to be framework-agnostic.

The last thing to fix (I believe) is the GenerateIcuData task, which is failing because the Icu4c*.Lib .props isn't being included for some reason.

* Replaced TargetFrameworks global property with TargetFramework in each project as needed.

This was necessary to build in .NET Standard. With the global property, some .props files weren't being loaded (e.g. Icu4c.Win.Fw.Lib.props), causing build errors in Core. On removal, all .props load as expected, and three projects required a TargetFramework to build. The original comment mentions needing TargetFrameworks, but it's not clear if this was because of the three projects (which I fixed) or something else.

* Changed SIL.WritingSystems reference in Core project to star pattern to fix TestHelper build.

Due to WarningsAsErrors in Directory.build.props

* Removed target framework folder from build output path.

This was necessary with the move to .NET Standard, because the libraries were being built to a different folder (netstandard2.0) than the executables (net461). This was messing up the various custom build tasks (mainly in .Core) that involved copying assemblies to the build folder. At this point all unit tests (Windows) are passing.

* Made the .Utils.Tests build dependent on .Core

This makes all test projects dependent on .Core, since .Utils.Tests is used by all of them. This change was necessary to fix the build for test projects when .Core hasn't already been built. Beforehand, as projects built asynchronously, System.CodeDom for .NET Standard would be copied to the artifacts folder (by .Build.Tasks) initially, but as the build for .Core followed, which requires that lib for its GenerateKernelCs build task, the build for .Utils.Tests would overwrite the System.CodeDom lib with its .NET Framework 4.6.1 copy. This was causing GenerateKernelCs to fail, unless of course .Core was built separately from the Test lib's. It should be noted that upgrading the Test lib's to .NET 6 does not fix the issue -- it appears that GenerateKernelCs can only use the .NET Standard 2.0 lib. But isn't that the whole point of .NET Standard?

* Updated IDLImporter to v2.0.1-beta0009.

This resolves the "first-time" custom build task exception, which was preventing builds from succeeding on the server.

* Removed a few lingering references to the .iip file.

I missed this, because running Clean Solution doesn't actually clean the solution. It leaves obj folders (including old, cached files) untouched.

* Removed "net461" from a build task only executed when packaging.

Hopefully this fixes the build server error.

* Removing NUnit package reference to try to fix tests in TeamCity

* Adding reference to Mono .NET Standard to fix some Linux test errors.

* Re-added reference to icu.net in Core.csproj.

This had been removed by the .NET Standard migration tool for some reason.

* Extracted color hex to a const in WritingSystemServices.cs

* Removing net461 from SilLCModelCoreLibPath in Core .props file.

I'm not sure if this is going to work. Currently, for consumers of this package, the build fails, searching for the .dll.config in the .../lib/net461/ path. But the .dll.config, which appears to use this property later in this changed file, isn't in .../lib/net461/ -- it's in .../lib/, and the .dll itself is in .../lib/netstandard2.0/. And I don't see anywhere where that's controlled, so if msbuild is expecting .dll.config and its .dll to be in the same folder, this won't work, either.

* In Build.Tasks, moved the packaged third-party tools back into an output path that specifies the project framework.

* Replaced NHunspell.Patched package with .csproj Include.

It appears that the only thing that .Patched adds is the copying of the two C++ lib's for NHunspell required at runtime, which was originally solved by creating a .Patched package with a .targets file. Adding an entirely new package (unless there's something else it does) for this one action isn't great. This must have been intended to replace the <package>/Tools/install.ps1 script in NHunspell that is no longer run, having been phased out of the msbuild/nuget process. The new solution accomplishes the same copy action without the need for a script or a separate package.

* Using Color per PR comment.

* Removing SIL.Core.Desktop reference from LCModel.Core.

This appears to be unused -- CustomIcuFallbackTests still pass, and that was the only reference to it. This is one less dependency to worry about when upgrading things (outside LCM) to .NET Standard etc., but it's also good cleanup for LCM.

* Setting MsBuildCommand to "dotnet build" for .NET Core.

Per https://sil-language-software.slack.com/archives/CGDKZ7NTV/p1654076175738199, I was apparently able to "dotnet build" locally without this, but I don't see how. Could've been totally off that day. This change allows dev's to run "dotnet build -c Release src/SIL.LCModel/SIL.LCModel.csproj" from cmd, which is what the GHA workflow is doing. It's necessary, because a typical .NET SDK install doesn't have an actual msbuild.exe, so the custom steps to build GenerateKernelCs.proj and GenerateModel.proj would fail. This change technically assumes you've got "dotnet.exe" in your PATH.

* Upgrading the deprecated CodeTaskFactory to RoslynCodeTaskFactory.

The latter is cross-platform, where the former isn't. See dotnet/msbuild#3175 for more information. Note that this functionality requires VS 15.8 or later. This change allows "dotnet build -c Release" to pass from cmd (it is a little flaky, just like the IDE build, but re-running enough times will fix it).

Co-authored-by: Jason Naylor <jasonleenaylor@users.noreply.github.com>
jasonleenaylor pushed a commit to sillsdev/liblcm that referenced this pull request Jul 7, 2022
The latter is cross-platform, where the former isn't. See dotnet/msbuild#3175 for more information. Note that this functionality requires VS 15.8 or later. This change allows "dotnet build -c Release" to pass from cmd (it is a little flaky, just like the IDE build, but re-running enough times will fix it).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants