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

Per-RuntimeIdentifier packaging doesn't allow overriding build output path in nupkg #217

Closed
andrew-boyarshin opened this issue Mar 22, 2020 · 4 comments · Fixed by #218
Closed

Comments

@andrew-boyarshin
Copy link
Contributor

andrew-boyarshin commented Mar 22, 2020

<!-- Include the runtimes files -->
<None Include="@(_BuildOutputInPackageWithRid)" PackagePath="runtimes/%(_BuildOutputInPackageWithRid.Rid)/lib/%(_BuildOutputInPackageWithRid.TargetFramework)" Pack="true" />
<None Include="@(_TargetPathsToSymbolsWithRid)" PackagePath="runtimes/%(_TargetPathsToSymbolsWithRid.Rid)/lib/%(_TargetPathsToSymbolsWithRid.TargetFramework)" Pack="true" />

This feels too restrictive. I'm not sure I can override this without overriding the whole target (and that requires dropping Sdk property from <Project /> and doing <Import Sdk="..." /> manually), or patching another target in GenerateNuspecDependsOn with <None Update="" />. nevermind, found a simple workaround, see the first comment below.

Proposal: extract path template to a property and allow simpler override.

Additional notes

The original issue title was Per-RuntimeIdentifier packaging doesn't respect BuildOutputTargetFolder, but then the whole property value is different from a traditional path under /lib or /tools...

It'd be great to have such things (ItemGroups) under some condition (like '$(ExtrasIncludeDefaultProjectBuildOutputInPack)' != 'false'), so that when such issues arise it would be possible simpler to hackaround fix it locally by setting property (in my example to false) and adding items manually.

Stumbled upon this while working on SharpGen, it provides its own MSBuild tasks, so they should be in tools directory for them to be dev-only deps. ref

@andrew-boyarshin
Copy link
Contributor Author

andrew-boyarshin commented Mar 22, 2020

My final version of workaround:

  <PropertyGroup>
    <GenerateNuspecDependsOn>$(GenerateNuspecDependsOn);PatchPackagePath</GenerateNuspecDependsOn>
  </PropertyGroup>

  <Target Name="PatchPackagePath">

    <ItemGroup>
      <_PackageFilesToPatch Include="@(_PackageFiles->WithMetadataValue('BuildAction', 'None')->WithMetadataValue('Pack', 'true')->HasMetadata('TargetFramework')->HasMetadata('Rid'))" />
      <_PackageFilesToPatch Remove="@(_PackageFilesToPatch)" Condition="'%(TargetFramework)' == '' or '%(Rid)' == ''" />
      <_PackageFiles Remove="@(_PackageFilesToPatch)" />
      <_PackageFilesToPatch Update="@(_PackageFilesToPatch)" PackagePath="tools/%(TargetFramework)/%(Rid)" />
      <_PackageFiles Include="@(_PackageFilesToPatch)" />
      <_PackageFilesToPatch Remove="@(_PackageFilesToPatch)" />
    </ItemGroup>

  </Target>

For anyone reading this and willing to understand the motivation behind these lines:
I didn't need to patch another target on GenerateNuspecDependsOn because appending your own target places it in exactly the right place, after the inputs were collected, but before they are immutable.
Had to create _PackageFilesToPatch to make it filterable by possibly non-existant metadatas TargetFramework and Rid (MSBuild evaluates condition even on items filtered out by WithMetadataValue and HasMetadata).
I didn't forget to clean up too (the last line).

I love MSBuild. place for an "unlimited power" Palpatine

@clairernovotny
Copy link
Collaborator

Is there a PR you'd suggest to make this generic/flexible?

@andrew-boyarshin
Copy link
Contributor Author

@clairernovotny I'll try to think of the most flexible approach to this and make a PR. Can't guarantee I'll do it, though. I'm already on the 5th level of the dependency chain (Project->Framework->Bindings->SharpGen->MSBuild SDK), the dev cycle gets more difficult by the minute.

@andrew-boyarshin
Copy link
Contributor Author

After #218 this can still be improved: modify Impl to take some new ItemSpec (abusing the fact that items in MSBuild preserve order), replace placeholders (like <RID> -> %(Rid)), join into string with / separator via @(..., '/'). But I believe it's me overthinking, so my PR does much simpler version which still enables full customization.

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

Successfully merging a pull request may close this issue.

2 participants