Skip to content

Remove BuildX64 target #41040

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

Merged
merged 1 commit into from
Apr 6, 2022
+0 −18
Merged

Remove BuildX64 target #41040

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 0 additions & 9 deletions src/Servers/IIS/IIS/test/testassets/InProcessNewShimWebSite/InProcessNewShimWebSite.csproj
View file
Original file line number Diff line number Diff line change
@@ -63,13 +63,4 @@
</PackageReference>
<Reference Include="xunit.assert" />
</ItemGroup>

<!-- Repeat Build target for win-x64 to allow later Publish w/ NoBuild=true. -->
<Target Name="BuildX64"
BeforeTargets="Build"
Condition=" '$(RuntimeIdentifier)' != 'win-x64' AND '$(TargetArchitecture)' != 'arm' ">
<MSBuild Projects="$(MSBuildProjectFullPath)"
Properties="RuntimeIdentifier=win-x64;ReferenceTestTasks=false"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest adding BuildProjectReferences=false to these Properties instead of removing the whole target. Same in the other project

Copy link
Member Author

Choose a reason for hiding this comment

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

Why is this target necessary at all given that all the tests are passing?

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right, these two <Target/>s are now redundant and cause problems.

Took a bit to find the change that made these <Target/>s unnecessary. They were mandatory back when I did #39998 because the Publish target was only run in the Helix build step and that step uses -noBuild. Publish fails if the project hasn't yet been built (in this case, for the $(RuntimeIdentifier) of interest).

This changed when I did #40379 because the CopyAssets target in FunctionalTest.props now runs in the prior build step, without -noBuild.

Targets="Build" />
</Target>
</Project>
9 changes: 0 additions & 9 deletions src/Servers/IIS/IIS/test/testassets/InProcessWebSite/InProcessWebSite.csproj
View file
Original file line number Diff line number Diff line change
@@ -34,13 +34,4 @@
<Reference Include="Microsoft.Extensions.Logging.Console" />
<Reference Include="xunit.assert" />
</ItemGroup>

<!-- Repeat Build target for win-x64 to allow later Publish w/ NoBuild=true. -->
<Target Name="BuildX64"
BeforeTargets="Build"
Condition=" '$(RuntimeIdentifier)' != 'win-x64' AND '$(TargetArchitecture)' != 'arm' ">
<MSBuild Projects="$(MSBuildProjectFullPath)"
Properties="RuntimeIdentifier=win-x64;ReferenceTestTasks=false"
Targets="Build" />
</Target>
</Project>