Skip to content

convert gentracesources to tt #598

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

Merged
merged 21 commits into from
May 6, 2019

Conversation

stevenbrix
Copy link
Contributor

@stevenbrix stevenbrix commented Apr 16, 2019

This PR updates the WpfArcadeSdk to move our gentracesources.pl and gentracestrings.pl to using design time t4 generation. Instead of these files being generated into $(IntermediateOutputPath) they are now put under $(WpfSourceDir), and are easily discoverable in the project files.

There are a few caveats in order to get this to work:

  1. We have to ensure BuildDependsOn variable that T4 targets update doesn't get overwritten. So with SDK style projects, we have to manually import the Sdk.targets and then include the T4 targets after.
  2. I wanted to decouple the GenTraceSources and GenAvMessages (similar to how we had 2 separate perl scripts before). In order to do this, those targets can't include the T4 targets, since then you get MSBuild authoring errors about the same target being imported twice.

Below is an example of how WindowsBase uses these targets. PresentationFramework and PresentationCore are similar, only without the GenTraceSources.targets import.

  <Import Project="Sdk.targets" Sdk="Microsoft.NET.Sdk" />
  <Import Project="$(WpfCodeGenDir)AvTrace\GenTraceSources.targets" />
  <Import Project="$(WpfCodeGenDir)AvTrace\GenAvMessages.targets" />
  <Import Project="$(WpfCodeGenDir)DesignTimeTextTemplating.targets" />

There will be an internal PR that shows how these changes are consumed. The goal here is that we will continue to move more and more of our codegen scripts to T4 templates, where we should be able to use this as a starting point for completing that work.

Copy link
Member

@vatsan-madhavan vatsan-madhavan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Need a little time to review - will get to as soon as possible after working through some preview 5 deliverables.

Would be great to get @rladuca's input as well.

Copy link
Member

@rladuca rladuca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, just some spelling/wording nits.

As a general suggestion, the comments you spit out in the T4 for some functions/properties should be XML style "///" comments.

The following document describes how code generation in this repo works. The goal is to have all our code generation done through the use of T4 text generation. See the offical [Visual Studio T4 documentation](https://docs.microsoft.com/en-us/visualstudio/modeling/design-time-code-generation-by-using-t4-text-templates?view=vs-2019) for more information.

## Design-Time vs Run-Time T4 templates
Currently, we are evaluating the use of design-time text templates. This gives us the ability to simply add the templates and associated targets to the build, without the need of maintaining a separate tool to do run-time generation. Including the `Microsoft.TextTemplating.targets` requires us to manually import Sdk.Targets because it needs to be imported after Sdk.targets. This causes the `BuildDependsOn` variable, which is modified by the T4 targets, to be overwritten, so the `TransformAll` target doesn’t run before the Build target. The boilerplait for including design-time templates has been encapsulated in the `$(WpfCodeGenDir)DesignTimeTextTemplating.targets` file, so the pattern for enabling these in a project looks like this:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Importing Sdk.Targets because it needs to be imported after Sdk.Targets" is phrased oddly.

I think what you want to say is more like "Microsoft.TextTemplating.targets needs to be imported after Sdk.Targets, therefore Sdk.Targets must be manually imported."

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also SDK.Targets probably should be in backticks as well.

Copy link
Contributor Author

@stevenbrix stevenbrix May 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rewording to:

When using the SDK-style project format, including the Microsoft.TextTemplating.targets requires us to manually import Sdk.Targets because the BuildDependsOn variable, which is modified by the T4 targets, would otherwise be overwritten by the automatic inclusion of Sdk.targets

@stevenbrix stevenbrix force-pushed the dev/stevenbrix/convertGenTraceSourcesToTT branch from 0b7e0a9 to 044170c Compare May 6, 2019 23:03
@stevenbrix stevenbrix merged commit 211a557 into master May 6, 2019
@stevenbrix stevenbrix deleted the dev/stevenbrix/convertGenTraceSourcesToTT branch May 28, 2019 19:52
@ghost ghost locked as resolved and limited conversation to collaborators Apr 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants