-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Add more properties controlling service reference code generation #10641
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
Changes from all commits
e219a70
2ad5939
58b1ed4
f1f5f2f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,35 +1,60 @@ | ||
<?xml version="1.0" encoding="utf-8" standalone="no"?> | ||
<Project> | ||
<PropertyGroup> | ||
<_ApiDescriptionTasksAssemblyTarget | ||
Condition="'$(MSBuildRuntimeType)' == 'Core'">netstandard2.0</_ApiDescriptionTasksAssemblyTarget> | ||
<_ApiDescriptionTasksAssemblyTarget | ||
Condition="'$(MSBuildRuntimeType)' != 'Core'">net461</_ApiDescriptionTasksAssemblyTarget> | ||
<_ApiDescriptionTasksAssemblyPath>$(MSBuildThisFileDirectory)/../tasks/$(_ApiDescriptionTasksAssemblyTarget)/Microsoft.Extensions.ApiDescription.Tasks.dll</_ApiDescriptionTasksAssemblyPath> | ||
<_ApiDescriptionTasksAssemblyTarget /> | ||
<_ApiDescriptionClientAssemblyTarget | ||
Condition="'$(MSBuildRuntimeType)' == 'Core'">netstandard2.0</_ApiDescriptionClientAssemblyTarget> | ||
<_ApiDescriptionClientAssemblyTarget | ||
Condition="'$(MSBuildRuntimeType)' != 'Core'">net461</_ApiDescriptionClientAssemblyTarget> | ||
<_ApiDescriptionClientAssemblyPath>$(MSBuildThisFileDirectory)/../tasks/$(_ApiDescriptionClientAssemblyTarget)/Microsoft.Extensions.ApiDescription.Client.dll</_ApiDescriptionClientAssemblyPath> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this really relevant anymore? MSBuild supports netstandard2.0 everywhere .NET Core runs. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the assembly has any dependencies that aren't in netstandard as implemented in .NET 4.7.2, I'd leave this in. Netstandard task assemblies are probably much more viable now that VS is on 4.7.2, but they've historically been a problem and splitting into core and full implementations has been required. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm mentioning it because we've been doing ns2.0 in Razor for a while now with no problems. Since this is new code it should be fine as long as we test it in VS right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, as long as it works in VS, command line msbuild.exe, and command line msbuild.exe from a Build Tools install of VS, you should be good to go. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've added this to #4923 since the extra bits do no harm. This is all an implementation detail and it'll take time to do the necessary testing… |
||
<_ApiDescriptionClientAssemblyTarget /> | ||
</PropertyGroup> | ||
<UsingTask TaskName="GetCurrentItems" AssemblyFile="$(_ApiDescriptionTasksAssemblyPath)" /> | ||
<UsingTask TaskName="GetFileReferenceMetadata" AssemblyFile="$(_ApiDescriptionTasksAssemblyPath)" /> | ||
<UsingTask TaskName="GetCurrentItems" AssemblyFile="$(_ApiDescriptionClientAssemblyPath)" /> | ||
<UsingTask TaskName="GetFileReferenceMetadata" AssemblyFile="$(_ApiDescriptionClientAssemblyPath)" /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. random observation: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added the suggestion to #4923 |
||
|
||
<!-- | ||
Settings users may update as they see fit. | ||
--> | ||
<PropertyGroup> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @glennc do you have any input on the names of these user-visible properties? |
||
<OpenApiDefaultGeneratorOptions Condition="'$(OpenApiDefaultGeneratorOptions)' == ''"></OpenApiDefaultGeneratorOptions> | ||
<!-- | ||
Options added to the code generator command line by default. Provides the default %(Options) metadata of | ||
@(OpenApiReference) and @(OpenApiProjectReference) items. | ||
--> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm always really happy to see quality docs in MSBuild 👍 |
||
<OpenApiGenerateCodeOptions Condition="'$(OpenApiGenerateCodeOptions)' == ''"></OpenApiGenerateCodeOptions> | ||
|
||
<!-- | ||
If 'true' (the default), generate code for @(OpenApiReference) and @(OpenApiProjectReference) items before the | ||
BeforeCompile target. | ||
|
||
If 'false', the 'GenerateOpenApiCode' target is not part of the build (by default) but can run when explicitly | ||
referenced. That is, the target may be invoked from the command line or tied in through another target. | ||
--> | ||
<OpenApiGenerateCodeOnBuild Condition="'$(OpenApiGenerateCodeOnBuild)' == ''">true</OpenApiGenerateCodeOnBuild> | ||
|
||
<!-- | ||
If 'true' (the default), generate code for @(OpenApiReference) and @(OpenApiProjectReference) items during | ||
design-time builds. Otherwise, generate code only during a full build. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you're going with the name There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thought about it but the sense is a bit different. @glennc and I decided this one was fine as-is. Could of course change it if users disagree. |
||
--> | ||
<OpenApiGenerateCodeAtDesignTime | ||
Condition="'$(OpenApiGenerateCodeAtDesignTime)' == ''">true</OpenApiGenerateCodeAtDesignTime> | ||
|
||
<!-- | ||
If 'true', will generate code for OpenApiReference items during design-time builds. Otherwise, generate code only | ||
for output files that do not yet exist during design-time builds. | ||
If 'true' (the default), build projects referenced in @(OpenApiProjectReference) items before retrieving that | ||
project's Open API documents list (or generating code). Recommend setting this to 'false' when the referenced | ||
projects do not share target framework values (avoiding build errors and / or warnings). | ||
|
||
If 'false', ensure the referenced projects build before this one in the solution or through other means. IDEs may | ||
be confused about the project dependency graph in this case. | ||
--> | ||
<OpenApiGenerateAtDesignTime Condition="'$(OpenApiGenerateAtDesignTime)' == ''">true</OpenApiGenerateAtDesignTime> | ||
<OpenApiBuildReferencedProjects | ||
Condition="'$(OpenApiBuildReferencedProjects)' == ''">true</OpenApiBuildReferencedProjects> | ||
|
||
<!-- | ||
$(OpenApiDefaultOutputDirectory) value is interpreted relative to the project folder, unless already an | ||
absolute path. | ||
Default folder to place code generated from Open API documents. Value is interpreted relative to the project | ||
folder, unless already an absolute path. Part of the default %(OutputPath) metadata of @(OpenApiReference) and | ||
@(OpenApiProjectReference) items. | ||
--> | ||
<OpenApiDefaultOutputDirectory | ||
Condition="'$(OpenApiDefaultOutputDirectory)' == ''">$(BaseIntermediateOutputPath)</OpenApiDefaultOutputDirectory> | ||
<OpenApiDefaultOutputDirectory>$([MSBuild]::EnsureTrailingSlash('$(OpenApiDefaultOutputDirectory)'))</OpenApiDefaultOutputDirectory> | ||
<OpenApiCodeDirectory | ||
Condition="'$(OpenApiCodeDirectory)' == ''">$(BaseIntermediateOutputPath)</OpenApiCodeDirectory> | ||
</PropertyGroup> | ||
|
||
<!-- | ||
|
@@ -42,24 +67,28 @@ | |
<OpenApiReference> | ||
<!-- Name of the class to generate. Defaults to match filename in %(OutputPath). --> | ||
<ClassName /> | ||
|
||
<!-- | ||
Code generator to use. Required and must end with "CSharp" or "TypeScript" (the currently-supported target | ||
languages) unless %(OutputPath) is set. Builds will invoke a target named "Generate%(CodeGenerator)" to do | ||
actual code generation. | ||
--> | ||
<CodeGenerator>NSwagCSharp</CodeGenerator> | ||
|
||
<!-- Namespace to contain generated class. Default is $(RootNamespace). --> | ||
<Namespace /> | ||
|
||
<!-- | ||
Options to pass to the code generator target then (likely) added to a tool's command line. Value is passed | ||
along to the code generator but otherwise unused in this package. | ||
--> | ||
<Options>$(OpenApiDefaultGeneratorOptions)</Options> | ||
<Options>$(OpenApiGenerateCodeOptions)</Options> | ||
|
||
<!-- | ||
Path to place generated code. Code generator may interpret path as a filename or directory. Default filename or | ||
folder name is %(Filename)Client.[cs|ts]. Filenames and relative paths (if explicitly set) are combined with | ||
$(OpenApiDefaultOutputDirectory). Final value (depending on $(OpenApiDefaultOutputDirectory)) is likely to be | ||
a path relative to the client project. | ||
$(OpenApiCodeDirectory). Final value (depending on $(OpenApiCodeDirectory)) is likely to be a path relative to | ||
the client project. | ||
--> | ||
<OutputPath /> | ||
</OpenApiReference> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,21 +2,18 @@ | |
<Project> | ||
<!-- Internal settings. Not intended for customization. --> | ||
<PropertyGroup> | ||
<GenerateOpenApiReferenceCodeDependsOn> | ||
_GetMetadataForOpenApiReferences; | ||
_GenerateOpenApiReferenceCode; | ||
_CreateCompileItemsForOpenApiReferences | ||
</GenerateOpenApiReferenceCodeDependsOn> | ||
<GenerateServiceFileReferenceCodeDependsOn> | ||
<GenerateOpenApiCodeDependsOn> | ||
_GenerateErrorsForOldItems; | ||
_CreateOpenApiReferenceItemsForOpenApiProjectReferences; | ||
GenerateOpenApiReferenceCode | ||
</GenerateServiceFileReferenceCodeDependsOn> | ||
_GetMetadataForOpenApiReferences; | ||
_GenerateOpenApiCode; | ||
_CreateCompileItemsForOpenApiReferences | ||
</GenerateOpenApiCodeDependsOn> | ||
</PropertyGroup> | ||
|
||
<!-- OpenApiProjectReference support. --> | ||
|
||
<ItemGroup> | ||
<ItemGroup Condition=" '$(OpenApiBuildReferencedProjects)' == 'true' "> | ||
<!-- | ||
Do not change %(ReferenceOutputAssembly) if project contains duplicate @(ProjectReference) and | ||
@(OpenApiProjectReference) items. | ||
|
@@ -30,19 +27,16 @@ | |
</ProjectReference> | ||
</ItemGroup> | ||
|
||
<Target Name="_CreateOpenApiReferenceItemsForOpenApiProjectReferences" | ||
AfterTargets="ResolveProjectReferences" | ||
Condition="'$(DesignTimeBuild)' != 'true' AND '$(BuildingProject)' == 'true'"> | ||
<Target Name="_CreateOpenApiReferenceItemsForOpenApiProjectReferences" DependsOnTargets="ResolveProjectReferences"> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It really doesn't matter because everything in this target no-ops when there's nothing worth doing. Let me know if you feel differently There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Simple is better, I agree with Doug. |
||
<ItemGroup> | ||
<_Temporary Remove="@(_Temporary)" /> | ||
</ItemGroup> | ||
|
||
<MSBuild Targets="OpenApiGetDocuments" | ||
Projects="@(ProjectReferenceWithConfiguration)" | ||
Condition="'%(ProjectReferenceWithConfiguration.OpenApiReference)' == 'true'" | ||
Properties="%(ProjectReferenceWithConfiguration.SetConfiguration); %(ProjectReferenceWithConfiguration.SetPlatform); %(ProjectReferenceWithConfiguration.SetTargetFramework)" | ||
BuildInParallel="$(BuildInParallel)" | ||
Projects="@(OpenApiProjectReference)" | ||
RebaseOutputs="true" | ||
RemoveProperties="%(ProjectReferenceWithConfiguration.GlobalPropertiesToRemove);TargetFrameworks;RuntimeIdentifier"> | ||
RemoveProperties="Configuration;Platform;RuntimeIdentifier;TargetFramework;TargetFrameworks"> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @rainersigwald is removing these properties sufficient? And, am I missing something important in the shift from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd definitely want to keep something like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great suggestion! I added this to #4923 because I have other bits to do short-term. |
||
<Output TaskParameter="TargetOutputs" ItemName="_Temporary" /> | ||
</MSBuild> | ||
|
||
|
@@ -62,7 +56,7 @@ | |
<GetFileReferenceMetadata Inputs="@(OpenApiReference)" | ||
Extension="$(DefaultLanguageSourceExtension)" | ||
Namespace="$(RootNamespace)" | ||
OutputDirectory="$(OpenApiDefaultOutputDirectory)"> | ||
OutputDirectory="$(OpenApiCodeDirectory)"> | ||
<Output TaskParameter="Outputs" ItemName="_Temporary" /> | ||
</GetFileReferenceMetadata> | ||
|
||
|
@@ -79,17 +73,16 @@ | |
</GetCurrentItems> | ||
</Target> | ||
|
||
<Target Name="_InnerGenerateOpenApiReferenceCode" DependsOnTargets="_GetCurrentOpenApiReference;$(GeneratorTarget)" /> | ||
<Target Name="_InnerGenerateOpenApiCode" DependsOnTargets="_GetCurrentOpenApiReference;$(GeneratorTarget)" /> | ||
|
||
<Target Name="_GenerateOpenApiReferenceCode" | ||
Condition="$(OpenApiGenerateAtDesignTime) OR ('$(DesignTimeBuild)' != 'true' AND '$(BuildingProject)' == 'true')" | ||
<Target Name="_GenerateOpenApiCode" | ||
Condition="$(OpenApiGenerateCodeAtDesignTime) OR ('$(DesignTimeBuild)' != 'true' AND '$(BuildingProject)' == 'true')" | ||
Inputs="@(OpenApiReference)" | ||
Outputs="%(OutputPath)"> | ||
<MSBuild BuildInParallel="$(BuildInParallel)" | ||
Projects="$(MSBuildProjectFullPath)" | ||
<MSBuild Projects="$(MSBuildProjectFullPath)" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this idea with this call that we only want to call this once even with multi-targeting? If that's the case should we have a comment? Basically I'm not sure why this is a separate invocation of MSBuild, a comment would help. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this is about running things once. And, sure, I'll add a comment |
||
BuildInParallel="$(BuildInParallel)" | ||
Properties="GeneratorTargetPath=%(OpenApiReference.OutputPath);GeneratorTarget=Generate%(CodeGenerator);GeneratorMetadata=%(SerializedMetadata)" | ||
RemoveProperties="TargetFrameworks" | ||
Targets="_InnerGenerateOpenApiReferenceCode" /> | ||
Targets="_InnerGenerateOpenApiCode" /> | ||
</Target> | ||
|
||
<Target Name="_CreateCompileItemsForOpenApiReferences" Condition="'@(OpenApiReference)' != ''"> | ||
|
@@ -133,17 +126,26 @@ | |
</ItemGroup> | ||
</Target> | ||
|
||
<Target Name="GenerateOpenApiReferenceCode" DependsOnTargets="$(GenerateOpenApiReferenceCodeDependsOn)" /> | ||
|
||
<!-- Tie above code generation steps into the build. --> | ||
<!-- Inform users of breaking changes in this file and Microsoft.Extensions.ApiDescription.Client.props. --> | ||
|
||
<Target Name="_GenerateErrorsForOldItems"> | ||
<Error Condition="'@(ServiceProjectReference)' != ''" Text="ServiceProjectReference items are no longer supported." /> | ||
<Error Condition="'@(ServiceUriReference)' != ''" Text="ServiceUriReference items are no longer supported." /> | ||
<Error Condition="'@(ServiceFileReference)' != ''" Text="ServiceFileReference items are no longer supported." /> | ||
</Target> | ||
|
||
<Target Name="GenerateServiceFileReferenceCode" | ||
<!-- Main code generation entry point. --> | ||
|
||
<Target Name="GenerateOpenApiCode" DependsOnTargets="$(GenerateOpenApiCodeDependsOn)" /> | ||
|
||
<!-- | ||
Unless this is an inner build or default timing is disabled, tie code generation into the build. Separate from | ||
GenerateOpenApiCode and not invoked from the ../buildMultiTargeting targets to prevent repeated code generation | ||
when multi-targeting. | ||
--> | ||
|
||
<Target Name="_TieInGenerateOpenApiCode" | ||
BeforeTargets="BeforeCompile" | ||
DependsOnTargets="$(GenerateServiceFileReferenceCodeDependsOn)" /> | ||
Condition=" '$(OpenApiGenerateCodeOnBuild)' == 'true' AND ('$(TargetFramework)' == '' OR '$(TargetFrameworks)' == '') " | ||
DependsOnTargets="GenerateOpenApiCode" /> | ||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
<?xml version="1.0" encoding="utf-8" standalone="no"?> | ||
<Project> | ||
<Import Project="../build/Microsoft.Extensions.ApiDescription.Client.props" /> | ||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,14 @@ | ||
<?xml version="1.0" encoding="utf-8" standalone="no"?> | ||
<Project> | ||
<Target Name="GenerateServiceFileReferenceCode" BeforeTargets="BeforeCompile"> | ||
<MsBuild Projects="$(MSBuildProjectFile)" | ||
Targets="GenerateServiceFileReferenceCode" | ||
<Target Name="GenerateOpenApiCode"> | ||
<MSBuild Projects="$(MSBuildProjectFile)" | ||
Targets="GenerateOpenApiCode" | ||
Properties="TargetFramework=$(TargetFrameworks.Split(';')[0])" | ||
RemoveProperties="TargetFrameworks;RuntimeIdentifier" /> | ||
RemoveProperties="RuntimeIdentifier" /> | ||
</Target> | ||
|
||
<Target Name="_GenerateOpenApiCode" | ||
BeforeTargets="BeforeCompile" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
Condition=" ''$(OpenApiGenerateCodeOnBuild)' == 'true' " | ||
DependsOnTargets="GenerateOpenApiCode" /> | ||
</Project> |
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 might be missing some necessary context here, but I don't really get the point of renaming an assembly containing tasks to not be suffixed with
.Tasks
.Is the metapoint here that we want to separate the tasks used for client-gen from the tasks used inside server-projects?
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.
That was my original point but I ended up not needing tasks in the server project and decided I preferred using the same name everywhere.