Skip to content
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.

Sign up for GitHub

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

Move some targets into a Directory.Build.targets file #39712

Merged
merged 3 commits into from
Jan 25, 2022
Merged

Move some targets into a Directory.Build.targets file #39712

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
529c8fc
Use regular artifacts/bin/ folders for IIS projects
dougbu Jan 23, 2022
9c58219
Move targets into a Directory.Build.targets file
dougbu Jan 24, 2022
20414d6
Do not "Use regular artifacts/bin/ folders for IIS projects"
dougbu Jan 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions src/Servers/IIS/IIS/test/testassets/Directory.Build.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<Project>
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory)..\, Directory.Build.targets))\Directory.Build.targets" />

<PropertyGroup>
<!-- Work around until we get a new WebSdk -->
Copy link
Member

Choose a reason for hiding this comment

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

@Tratcher any context about what this workaround comment is talking about?

Copy link
Member Author

Choose a reason for hiding this comment

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

I wondered about this too❕

Copy link
Member

Choose a reason for hiding this comment

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

LOL, no, but it happened back in 3.0 so it's probably safe to say we got a fresh WebSdk update since then 😁.
dougbu@a474a05

Copy link
Member Author

Choose a reason for hiding this comment

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

The current default (in Microsoft.NET.Sdk.Web.ProjectSystem.targets) is inprocess and this override may make sense still. Let's use this space to answer

  1. Should we update InProcessNewShimWebSite.csproj and InProcessWebSite.csproj instead of leaving this setting here❔
  2. Did I break anything by effectively removing this setting from NativeIISSample.csproj and ServerComparison.TestSites.csproj❔
  3. Does the setting have any impact in projects that import testsite.props but don't use Microsoft.NET.Sdk.Web❔
  4. What are the best hosting models for each of the affected projects❔

Copy link
Member Author

Choose a reason for hiding this comment

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

On (1), I suspect (given the names), the current setting is unhelpful for both InProcess*WebSite projects. Suggest we simply delete these lines and focus on the other projects.

On (2), "break" is probably too strong a word but I did change things. Good thing tests are still working.

<AspNetCoreHostingModel>OutOfProcess</AspNetCoreHostingModel>

<HasTestAssetProfile Condition="'$(TestAssetProfile)' != ''">true</HasTestAssetProfile>
<TestAssetOutputName>$(MSBuildProjectName)</TestAssetOutputName>
</PropertyGroup>

<Target Name="PreventProjectReferencesFromBuilding" BeforeTargets="BeforeResolveReferences" Condition="'@(TestAssetPublishProfile->Count())' != '0'">
<PropertyGroup>
<BuildProjectReferences Condition="'$(HasTestAssetProfile)' == 'true'">false</BuildProjectReferences>
</PropertyGroup>
</Target>

<Target Name="PrepareForTestAssetPublish" BeforeTargets="PrepareForPublish" Condition="'@(TestAssetPublishProfile->Count())' != '0'">
<PropertyGroup Condition="'$(HasTestAssetProfile)' == 'true'">
<PublishDir>$(PublishDir)\$(TestAssetOutputName)-$(TestAssetProfile)\</PublishDir>
</PropertyGroup>

<PropertyGroup Condition="'$(HasTestAssetProfile)' != 'true'">
<IsPublishable>false</IsPublishable>
<IsTransformWebConfigDisabled>true</IsTransformWebConfigDisabled>
</PropertyGroup>
</Target>

<Target Name="PublishTestsAsset" DependsOnTargets="Publish" Returns="@(PublishedTestAsset)">
<ConvertToAbsolutePath Paths="$(PublishDir)">
<Output TaskParameter="AbsolutePaths" PropertyName="PublishAbsoluteDir" />
</ConvertToAbsolutePath>

<ItemGroup>
<PublishedTestAsset Include="$(TestAssetOutputName)-$(TestAssetProfile)" Path="$(PublishAbsoluteDir)" />
</ItemGroup>
</Target>

<Target Name="PublishTestsAssets" Condition="'$(TestAssetProfile)' == '' AND '@(TestAssetPublishProfile->Count())' != '0'" Returns="@(PublishedTestAsset)">
<!-- Platform;PlatformTarget removal is to fix builds inside VS. -->
<MSBuild Projects="$(MSBuildProjectFullPath)"
Targets="PublishTestsAsset"
RemoveProperties="Platform;PlatformTarget"
Properties="PublishDir=$(PublishDir);TestAssetProfile=%(TestAssetPublishProfile.Identity);ReferenceTestTasks=false;%(TestAssetPublishProfile.Properties)">
<Output TaskParameter="TargetOutputs" ItemName="PublishedTestAsset" />
</MSBuild>
</Target>
</Project>
3 changes: 0 additions & 3 deletions src/Servers/IIS/IIS/test/testassets/InProcessNewShimWebSite/InProcessNewShimWebSite.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
<Project Sdk="Microsoft.NET.Sdk.Web">

<Import Project="..\..\..\..\build\testsite.props" />

<PropertyGroup>
<TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
<AssemblyName>InProcessWebSite</AssemblyName>
<TestAssetOutputName>InProcessNewShimWebSite</TestAssetOutputName>
Copy link
Member Author

Choose a reason for hiding this comment

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

Another nit: Setting matches the default.

<DefineConstants>FORWARDCOMPAT</DefineConstants>
<CompileUsingReferenceAssemblies>false</CompileUsingReferenceAssemblies>
<ImplicitUsings>disable</ImplicitUsings>
Expand Down Expand Up @@ -61,5 +59,4 @@
</PackageReference>
<Reference Include="xunit.assert" />
</ItemGroup>

</Project>
51 changes: 3 additions & 48 deletions src/Servers/IIS/build/testsite.props
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,10 @@
<Project>

<PropertyGroup>
<RuntimeIdentifiers>$(RuntimeIdentifiers);win-x64;win-x86</RuntimeIdentifiers>
<Platforms>x64;x86</Platforms>
<IISExpressAppHostConfig>$(MSBuildThisFileDirectory)applicationhost.config</IISExpressAppHostConfig>
<IISAppHostConfig>$(MSBuildThisFileDirectory)applicationhost.iis.config</IISAppHostConfig>
<PreserveCompilationContext>false</PreserveCompilationContext>
<!-- Work around until we get a new WebSdk -->
<AspNetCoreHostingModel>OutOfProcess</AspNetCoreHostingModel>
<HasTestAssetProfile Condition="'$(TestAssetProfile)' != ''">true</HasTestAssetProfile>
<TestAssetOutputName Condition="'$(TestAssetOutputName)' == ''">$(MSBuildProjectName)</TestAssetOutputName>
</PropertyGroup>

<Import Project="assets.props" />
Expand All @@ -31,9 +26,9 @@
<IISArguments>-h "$(IISAppHostConfig)"</IISArguments>

<AncmInProcessRHPath>aspnetcorev2_inprocess.dll</AncmInProcessRHPath>
<!-- TODO: use LocalDotNetRoot instead to find dotnet.exe (see reporoot/Directory.Build.props). Requires a fix to https://github.com/dotnet/aspnetcore/issues/7196 -->
<DotNetPath>$(RepoRoot).dotnet\dotnet.exe</DotNetPath>
<DotNetPath Condition="'$(NativePlatform)' != 'x64'">$(RepoRoot).dotnet\$(NativePlatform)\dotnet.exe</DotNetPath>

<DotNetPath>$(DotNetRoot)dotnet.exe</DotNetPath>
<DotNetPath Condition="'$(NativePlatform)' != 'x64'">$(DotNetRoot)$(NativePlatform)\dotnet.exe</DotNetPath>
</PropertyGroup>

<Target Name="CopyLaunchSettings" AfterTargets="CoreBuild">
Expand Down Expand Up @@ -81,44 +76,4 @@
DependsOnTargets="GeneratePublishDependencyFile;CopyFilesToPublishDirectory;PrepareInjectionApp">
<Exec Command="$(InjectDepsApp) &quot;$(PublishDepsFilePath)&quot; $(RuntimeIdentifier) " />
</Target>

<Target Name="PreventProjectReferencesFromBuilding" BeforeTargets="BeforeResolveReferences" Condition="'@(TestAssetPublishProfile->Count())' != '0'">
<PropertyGroup>
<BuildProjectReferences Condition="'$(HasTestAssetProfile)' == 'true'">false</BuildProjectReferences>
</PropertyGroup>
</Target>

<Target Name="PrepareForTestAssetPublish" BeforeTargets="PrepareForPublish" Condition="'@(TestAssetPublishProfile->Count())' != '0'">
<PropertyGroup Condition="'$(HasTestAssetProfile)' == 'true'">
<PublishDir>$(PublishDir)\$(TestAssetOutputName)-$(TestAssetProfile)\</PublishDir>
</PropertyGroup>

<PropertyGroup Condition="'$(HasTestAssetProfile)' != 'true'">
<IsPublishable>false</IsPublishable>
<IsTransformWebConfigDisabled>true</IsTransformWebConfigDisabled>
</PropertyGroup>
</Target>

<Target Name="PublishTestsAsset" DependsOnTargets="Publish" Returns="@(PublishedTestAsset)">
<ConvertToAbsolutePath Paths="$(PublishDir)">
<Output TaskParameter="AbsolutePaths" PropertyName="PublishAbsoluteDir" />
</ConvertToAbsolutePath>

<ItemGroup>
<PublishedTestAsset Include="$(TestAssetOutputName)-$(TestAssetProfile)" Path="$(PublishAbsoluteDir)" />
</ItemGroup>
</Target>

<Target Name="PublishTestsAssets" Condition="'$(TestAssetProfile)' == '' AND '@(TestAssetPublishProfile->Count())' != '0'" Returns="@(PublishedTestAsset)">

<!--
_InsidePublishTestsAssets is to avoid invoking this target recursively
Platform;PlatformTarget removal is fix builds inside VS -->
<MSBuild Projects="$(MSBuildProjectFullPath)"
Targets="PublishTestsAsset"
RemoveProperties="Platform;PlatformTarget"
Properties="PublishDir=$(PublishDir);TestAssetProfile=%(TestAssetPublishProfile.Identity);ReferenceTestTasks=false;%(TestAssetPublishProfile.Properties)">
<Output TaskParameter="TargetOutputs" ItemName="PublishedTestAsset" />
</MSBuild>
</Target>
</Project>
2 changes: 1 addition & 1 deletion src/Servers/testassets/Directory.Build.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory)..\, Directory.Build.props))\Directory.Build.props" />

<PropertyGroup>
<!-- Tests do not work on Helix or when bin/ directory is not in project directory due to undeclared dependency on test content. -->
<!-- Local testing does not work when bin/ directory is not in project directory due to deployer expectations. -->
Copy link
Member Author

Choose a reason for hiding this comment

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

nit: I don't think this was about "test content". Problem seems rooted in the various deployers.

<BaseOutputPath />
<OutputPath />
</PropertyGroup>
Expand Down