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.

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

WPF project is missing nuget imports for "nested" build #3483

Closed
ericstj opened this issue Jul 5, 2018 · 10 comments
Closed

WPF project is missing nuget imports for "nested" build #3483

ericstj opened this issue Jul 5, 2018 · 10 comments
Labels

Comments

@ericstj
Copy link
Member

ericstj commented Jul 5, 2018

Suppose I have a nuget package that includes targets that are necessary for compile.

WPF projects will create a copy of the project on the fly and build it. See https://referencesource.microsoft.com/#PresentationBuildTasks/BuildTasks/Microsoft/Build/Tasks/Windows/GenerateTemporaryTargetAssembly.cs,98

The problem is that the task which generates this project chooses a random file name without any way of predicting what it will be:
https://referencesource.microsoft.com/#PresentationBuildTasks/BuildTasks/Microsoft/Build/Tasks/Windows/GenerateTemporaryTargetAssembly.cs,144

Now when that nested project loads up, it will no longer pick up the generated props/targets files for NuGet packages because it has a different $(MSBuildProjectName).
https://github.com/Microsoft/msbuild/blob/master/src/Tasks/Microsoft.Common.props#L66
https://github.com/Microsoft/msbuild/blob/master/src/Tasks/Microsoft.Common.targets#L127

This is broken with desktop WPF projects using PackageReference today, and I suspect its something we'll need to fix for .NETCore v3.0 as well.

A couple thoughts:

  1. Allow a project to set the property used for this hook and default to MSBuildProjectName. Then old desktop projects could set whatever the new property is.
  2. If _TargetAssemblyProjectName is set, use this instead of MSBuildProjectName. It would seem that this was created explicitly for this purpose, but was only added in recent versions of .NET desktop. We could document this property so that projects using the old task can explicitly set it.

/cc @vatsan-madhavan-msft @dsplaisted

@dsplaisted
Copy link
Member

Is it feasible to fix the WPF targets to not generate a temporary copy of the project for this? Can it instead just do something similar to what we do for "inner" builds for multitargeted projects, where the project builds itself with some different global properties set?

@jtbrower
Copy link

jtbrower commented Jun 6, 2020

Now I finally know what is going on with my own builds. Up until today, it was just a random nuisance, but now it is preventing about 50% of my WPF projects from building.

Let me explain the problem I have and how I am contributing to that problem. I have a unique build system, I am one of those who loves to separate all of my build output files, I like to assure deterministic builds. All of my repositories work the same way, that is I have every project in each repository build its output into a common artifacts directory. For example \RepositoryName\artifacts. Under the artifacts directory I split the compiler output up into obj\bin\ext\pdb and globals. For the two directories that don't have obvious names (globals is the global nuget cache directory for the repo) and the (ext are the intermediate nuget files that are normally placed in the obj directory). This level of OCD really helps me when I am building scripts for the now 149 repositories that I am in control of. However, I have always had a random issue that I couldn't quite figure out. That is, for some WPF projects I would see directories that contained random characters appended to the end of them. Until today, it had always been on the "figure that out someday when I am bored" TODO list.

Today I upgraded from .Net 5.0 preview 5 to .Net 5.0 preview 7. Now I have quite a few WPF projects that will not build because the value of the MSBuildProjectName contains those random temporary characters at the time that I use it to help set the MSBuildProjectExtensionsPath, BaseIntermediateOutputPath, IntermediateOutputPath, OutputPath and OutDir. I have been using this approach for over a year now. Although I noticed those temporary directories once in a while, it rarely caused build failures. I don't know what it is about the .Net 5 preview 7 build that agitates the condition but I am going to need to figure out a work-around like removing any characters in project name suffixed with an underscore (since that is the consistent starting character I see).

I have included this part of my Directory.Build.props file just for reference. @dsplaisted, you asked two years ago if avoiding the temporary copy would fix this. I think that no matter what, the MSBuildProjectName variable should always be deterministic. It should always contain one value, the name of the project. Yes, technically it still does contain the name of the project, but its the unexpected temporary project. Its something that I never expected to occur and as far as I know there is no documentation on this on any of the msbuild help pages that educate us on msbuild properties such as the commonly used MSBuildProjectName property. Now that I know I cannot trust the value of MSBuildProjectName in WPF projects, I at least know where those temporary directories are coming from and I can find a work-around.

I know that I am doing some outside the box build steps, but it would be great if MSBuildProjectName always contained the actual project name. I guess I should vote yes for your #3497 because that's exactly part of what I am doing here.

Cheers, Jason

<Project InitialTargets="CheckEnvironment">

  <PropertyGroup Condition=" '$(ProjectArtifactDir)' == '' ">
    <ProjectArtifactDir>$(Configuration)\$(MSBuildProjectName)</ProjectArtifactDir>
  </PropertyGroup>

  <!-- SolutionDir was defined -->
  <PropertyGroup Condition=" '$(ArtifactsRoot)' == '' And ('$(SolutionDir)' != '*Undefined*' And '$(SolutionDir)' != '') ">
    <ArtifactsRoot>$(SolutionDir)</ArtifactsRoot>
    <ArtifactsRoot Condition="!HasTrailingSlash('$(ArtifactsRoot)')">$(ArtifactsRoot)\</ArtifactsRoot>
    <ArtifactsRoot>$(ArtifactsRoot)artifacts\</ArtifactsRoot>
  </PropertyGroup>

  <!-- SolutionDir was not defined -->
  <PropertyGroup Condition=" '$(ArtifactsRoot)' == '' And ('$(SolutionDir)' == '*Undefined*' Or '$(SolutionDir)' == '') ">
    <!-- Though 99% of our use cases require a solution, Benchmark DotNet causes this
    to be undefined due to its dynamic compilation. The work-around follows. -->

    <!-- If compiled without a solution. -->
    <ArtifactsRoot>$(MSBuildProjectDirectory)</ArtifactsRoot>
    <ArtifactsRoot Condition="!HasTrailingSlash('$(ArtifactsRoot)')">$(ArtifactsRoot)\</ArtifactsRoot>
    <ArtifactsRoot>$(ArtifactsRoot)artifacts\</ArtifactsRoot>
  </PropertyGroup>

  <!--
    Place the obj, bin and extension files at the root of the solution so
    cleaning is easy.
  -->
  <PropertyGroup>
    <AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath>

    <!-- https://docs.microsoft.com/en-us/visualstudio/msbuild/customize-your-build?view=vs-2019
    -->
    <MSBuildProjectExtensionsPath>$(ArtifactsRoot)ext\$(ProjectArtifactDir)</MSBuildProjectExtensionsPath>

    <!-- https://github.com/microsoft/msbuild/issues/3244 Bug requires us to set the Base path too. -->
    <BaseIntermediateOutputPath>$(ArtifactsRoot)obj\$(ProjectArtifactDir)</BaseIntermediateOutputPath>
    <IntermediateOutputPath>$(BaseIntermediateOutputPath)</IntermediateOutputPath>

    <OutputPath>$(ArtifactsRoot)bin\$(ProjectArtifactDir)</OutputPath>
    <OutDir>$(OutputPath)</OutDir>

  </PropertyGroup>

  <Target Name="CheckEnvironment">
    <Error Text="Configuration is not defined.  Use dotnet build '-c Debug' or '-c Release'." Condition=" '$(Configuration)' == '' " />
    <Message Importance="high" Text="SolutionDir not defined, bin/obj will be under $(ArtifactsRoot)" Condition=" '$(SolutionDir)' == '*Undefined*' Or '$(SolutionDir)' == ''" />
    <Message Importance="high" Text="PROJECT NAME $(MSBuildProjectName)" />
  </Target>

</Project>

@jtbrower
Copy link

jtbrower commented Jun 6, 2020

I have found a work-around that (at the moment) is more reliable for determining the true value of MSBuildProjectName.

<RealProjName>$([System.IO.Path]::GetFileName('$(MSBuildProjectDirectory)'))</RealProjName>

@vatsan-madhavan
Copy link
Member

We normalize project names like this in dotnet/wpf:

  <PropertyGroup>
    <!-- 
      Some WPF product assemblies, like System.Windows.Controls.Ribbon, require markup compilation.
      At present, a temporary project is created on the disk during markup-compilation with a name like 
          <ProjectName>_random_wpftmp.csproj
      Normalizing $(MSBuildProjectName) allows us to ensure that temporary projects of this nature are also
      correctly treated as IsShipping=true
    -->
    <NormalizedMSBuildProjectName Condition="!$(MSBuildProjectName.EndsWith('_wpftmp'))">$(MSBuildProjectName)</NormalizedMSBuildProjectName>
    <NormalizedMSBuildProjectName Condition="$(MSBuildProjectName.EndsWith('_wpftmp'))">$(MSBuildProjectName.SubString(0, $(MSBuildProjectName.IndexOf('_'))))</NormalizedMSBuildProjectName>

I would not recommend merging the output from foo_wpftmp_xxx.csproj and foo.csproj into the same obj, bin etc folders. They should be assigned different project-specific folders (as they are different projects). If one clobbers the outputs from the other, it will lead to unexpected results. They are not interchangeable with one another.

That said they can indeed share the same BaseIntermediateOutputPath etc. I’m fact dotnet/arcade has props/targets that achieves this for all .NET Core repos. See https://github.com/dotnet/arcade/blob/9811f06184cd2adae554f013ece07bece2a6c50e/src/Microsoft.DotNet.Arcade.Sdk/tools/ProjectLayout.props#L4 etc.

@jtbrower
Copy link

jtbrower commented Jun 6, 2020

@vatsan-madhavan thank you for the quick response. I now have about 13 years with WPF and until now I didn't know anything about these _xxx.csproj and I still do not understand what they are for. I would never willingly and knowingly combine the output from two project builds but knowing nothing about the reason for this temporary project, its easy to make this mistake.

Can you shed some light on the purpose for those? How can I spot which of my WPF libraries will be compiled this way? Where in my props file am I merging bin and obj files? Thanks again for the quick response.

@vatsan-madhavan
Copy link
Member

It is a used by the GenerateTemporaryTargetAssembly task during markup compilation.

https://github.com/dotnet/wpf/blob/05b6d65560c6ce839a9e375a3b7ef0bf64e8a5f0/src/Microsoft.DotNet.Wpf/src/PresentationBuildTasks/Microsoft/Build/Tasks/Windows/GenerateTemporaryTargetAssembly.cs#L43-L54

A copy of the WPF project is created after stripping out XAML etc references to build a preliminary version of the assembly, which is used to bootstrap types for markup compilation in preparation of “full” compilation of the assembly for real.

@jtbrower
Copy link

jtbrower commented Jun 7, 2020

@vatsan-madhavan I really appreciate you pointing out the fact that if I normalized the project name and used that normalized name to set all of the artifact output paths, that indeed would have placed the output from two projects into the same directory. Now that I understand that WPF projects are compiled by creating a temporary copy of the project, I won't make that mistake again.

I updated the section of the Directory.Build.props file that helps assure that the artifacts output is located at the root of the repository. I borrowed most of the logic from the Arcade props files you pointed me too and have included it below for anyone else that's interested. Note that I also stopped trying to put the third party package files into the separate ext directory and left them in obj. The main goal I had is accomplished, keep all binary files at the root of each of my repositories, and of course do this without causing subtle compilation issues.

For anyone that borrows from the following logic, all of the repositories I manage have their own Directory.Build.props file that defines the ArtifactsRoot and then includes this common section from a shared repo. The script will throw an error if ArtifactsRoot is not defined.

<Project InitialTargets="CheckEnvironment">

  <!-- The entire purpose of this file is to assure the compilation output is placed under a folder at the root of each repository. -->
  <PropertyGroup>
    <ArtifactsObjDir>$([MSBuild]::NormalizeDirectory('$(ArtifactsRoot)', 'obj'))</ArtifactsObjDir>
    <ArtifactsBinDir>$([MSBuild]::NormalizeDirectory('$(ArtifactsRoot)', 'bin'))</ArtifactsBinDir>
    <Platform Condition="'$(Platform)' == ''">AnyCPU</Platform>
    <PlatformName Condition="'$(PlatformName)' == ''">$(Platform)</PlatformName>
  </PropertyGroup>

  <!--
    Place the obj, bin and extension files at the root of the solution so
    cleaning is easy.
  -->
  <PropertyGroup>
    <OutDirName Condition="'$(OutDirName)' == ''">$(MSBuildProjectName)</OutDirName>

    <BaseOutputPath Condition="'$(BaseOutputPath)' == ''">$([System.IO.Path]::GetFullPath('$(ArtifactsBinDir)$(OutDirName)\'))</BaseOutputPath>
    <OutputPath Condition="'$(PlatformName)' == 'AnyCPU'">$(BaseOutputPath)$(Configuration)\</OutputPath>
    <OutputPath Condition="'$(PlatformName)' != 'AnyCPU'">$(BaseOutputPath)$(PlatformName)\$(Configuration)\</OutputPath>

    <BaseIntermediateOutputPath Condition="'$(BaseIntermediateOutputPath)' == ''">$([System.IO.Path]::GetFullPath('$(ArtifactsObjDir)$(OutDirName)\'))</BaseIntermediateOutputPath>
    <IntermediateOutputPath Condition="'$(PlatformName)' == 'AnyCPU'">$(BaseIntermediateOutputPath)$(Configuration)\</IntermediateOutputPath>
    <IntermediateOutputPath Condition="'$(PlatformName)' != 'AnyCPU'">$(BaseIntermediateOutputPath)$(PlatformName)\$(Configuration)\</IntermediateOutputPath>
  </PropertyGroup>

  <!--
    Both the Configuration and the ArtifactsRoot must be defined.
    ArtifactsRoot is defined in the Directory.Build.props at the root of every
    repo.  However some projects in each repo may skip including that props file.
    If they skip including the root props file but still include this one, they
    will see errors unless they or another props file defines the ArtifactsRoot
    and Configuration properties.
  -->
  <Target Name="CheckEnvironment">
    <Error Text="Configuration is not defined.  Use dotnet build '-c Debug' or '-c Release'." Condition=" '$(Configuration)' == '' " />
    <Error Text="ArtifactsRoot must be defined when including Output.Build.props. This is usually done by the Directory.Build.props file at the root of the repository." Condition=" '$(ArtifactsRoot)' == '*Undefined*' Or '$(ArtifactsRoot)' == ''" />
  </Target>
</Project>

@dsplaisted
Copy link
Member

A copy of the WPF project is created after stripping out XAML etc references to build a preliminary version of the assembly, which is used to bootstrap types for markup compilation in preparation of “full” compilation of the assembly for real.

If possible, I believe it would be much preferable to do this without generating a separate temporary project.

<MSBuild Projects="@(MSBuildProjectFullPath)"
         Targets="Build"\
         Properties="GenerateTemporaryTargetAssembly=True" />

Then the WPF targets would have logic that would strip out the appropriate references, adjust the intermediate and output paths, and whatever else is needed based on the GenerateTemporaryTargetAssembly property.

@jtbrower
Copy link

@dsplaisted I agree

@rainersigwald
Copy link
Member

This is being resolved in the WPF repo with dotnet/wpf#3585.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants