-
Notifications
You must be signed in to change notification settings - Fork 59
feat: Allow override of the generated files output path #228
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good. I'd like to ask for a few more things to finish this off:
- An example in
samples
with an appropriate check somewhere (either a target in csproj, or a build script with the check) that the files were actually saved to the other, specified directory. - Document the feature in the README, close to the current MSBuild extensibility documentation.
- Put an entry in CHANGELOG.md about this feature as an addition.
I think this should be enough to polish and merge this PR.
@@ -64,6 +64,8 @@ | |||
<CodeGenerationRoslynToolPath | |||
Condition="'$(GenerateCodeFromAttributesToolPathOverride)' != ''">$(GenerateCodeFromAttributesToolPathOverride)</CodeGenerationRoslynToolPath> | |||
<CodeGenerationRoslynToolPath>$(CodeGenerationRoslynToolPath.Trim())</CodeGenerationRoslynToolPath> | |||
<CodeGenerationRoslynFilesOutputPath |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'd prefer CodeGenerationRoslynToolOutputPath
.
I'd also like to suggest that maybe a warning should be raised (with a code to help suppression) when:
These two cases will result in files being overwritten and an undefined result in the end. |
Thanks for the feedback, I'll work on those changes. One question in the meantime: why does CGR generate an AssemblyAttributes file (.e.g .NETStandard,Version=v2.0.AssemblyAttributes.APNPx2rD.generated.cs). The file content is only the auto-generated banner and then:
But it seems that this file is picked up for generation again by CGR. So if the |
This will be the case anyway if you'll not exclude those files from default Compile include manually. This should also probably be highlighted in documentation. CGR runs before CoreCompile (at which time the AssemblyAttributes.cs is already generated), and it takes in all Compile items ( So, by default, if you set the Because every subsequent build will include the generated files in the Compile item by default, and we'll generate duplicates of those files, as you've noticed. Our compilation setup wasn't ever meant to have the files saved to the project directory. It's designed from the ground up as a per-Compilation runner. |
Ok, so all files are passed to CGR.Tool before the compile. But if a generator outputs, say, an empty class definition (and the generated class is therefore not attributed for code generation) it will get included in a subsequent call to CGR.Tool, but as it's not marked for generation it's not passed to a generator and nothing happens with it -- no new files are produced. Only if a generated file is attributed for code generation do we get this recursive .generated.generated.generated... problem. For example this happens with the Except for the |
Speculating here (no proof), but I'm pretty sure we create the new files no matter if there were any generators run or trigger attributes found. It may be the case that they get re-generated and overwritten. It all just adds to this being a totally not supported scenario. :( |
I think we could mitigate at least partially the infinite-regeneration scenario by changing slightly how we consume the Compile list:
@robkroll would you be interested in adding this in this PR? @AArnott do you think this'd work? |
Yes, I think that's promising @amis92. Another option, IIRC, is that we only generate for "all" source files right now because of an |
Allow override of the output directory where CGR.Tool writes generated files. Defaults to existing value
$(IntermediateOutputPath)
so no change for existing users.Partially fixes #95