Repository navigation
fix: publish/ ships the unmerged Plugin assembly; missing entirely for WorkflowActivity - #107
Merged
Merged
Conversation
…ublish/ folder exists BuildPluginLibraries invokes referenced Plugin projects with Targets="Build", but EnsurePluginAssemblyDataXml looks for the assembly under bin/<Config>/<TFM>/publish/, which a plain Build never creates - only Publish does. The dormant _PublishPluginAfterBuild hook that produces this folder only fired when PublishOnBuild=true, which nothing ever set. Default PublishOnBuild=true for ProjectType=Plugin (and, analogously, WorkflowActivity, which has the identical assumption feeding EnsureWorkflowActivityAssemblyDataXml but no publish hook at all) unless the consumer already set it explicitly. Fixing the missing publish/ folder alone isn't sufficient: _AssemblyMergePluginDependenciesAfterBuild (ILRepack merge) also hooks AfterTargets="Build", and AfterTargets lists are OR not AND, so anchoring the new publish hook on "_AssemblyMergePluginDependenciesAfterBuild;Build" would let it attach right after Build and run before the merge. Anchoring on the merge target alone (itself anchored on Build) fixes the ordering. Separately, the SDK's Publish pipeline copies the primary assembly from the intermediate obj/ output rather than the merged bin/ output, so the merged assembly is explicitly re-copied into the publish/ folder afterwards. Both fixes were verified experimentally against a minimal net8.0 project importing the real target files with a genuine ILRepack merge (see PR description).
Merged
4 tasks done
Rewrites the AfterTargets-ordering and obj/-vs-bin/-copy comments as plain documentation instead of a narrated verification log, and drops the ProjectType==Plugin/WorkflowActivity condition on the PublishOnBuild default - both files are only ever imported for that project type, and every other property default in them (IsPackable, ILRepackTargetsFile, etc.) is already unconditioned on ProjectType for the same reason. No target ordering, AfterTargets, or task logic changes.
TomProkop
changed the title
TomProkop
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Verified behavior
Tested by packing this branch and building a real Solution project (2 Plugin projects,
BuildInParallel="true", defaultnet472, noTargetFramework/SDK version pinned anywhere — matching normal consumer usage) against the actualBuildPluginLibrariespath, and comparing against the pre-fix code.Plugin:
Microsoft.PowerApps.MSBuild.Plugin(pinned1.48.2inDirectory.Build.props) already defaultsPublishOnBuild=trueon its own, so the dormant_PublishPluginAfterBuildtarget already ran today, before this PR. The actual, currently-shipping bug is that it's anchored on plainAfterTargets="Build"— the same anchor as_AssemblyMergePluginDependenciesAfterBuild(the ILRepack merge) — and is declared earlier in the file, so it deterministically runs before the merge, not after. Confirmed by hash: on the currentmaster, a Plugin project with a real merged dependency produces a 14,336-byte mergedbin/<Name>.dllnext to a 4,096-byte unmergedpublish/<Name>.dll— every build, silently missing all merged dependencies. This is not a hypothetical: it's what ships today wheneverAssemblyMergeSkipisn't set.WorkflowActivity: no equivalent Microsoft package sets a
PublishOnBuilddefault here, sopublish/is genuinely never created at all — the originalFileNotFoundException: Build not foundsymptom is real and reproducible for this project type.Fix
_AssemblyMerge*DependenciesAfterBuildinstead ofBuild, so it always runs after the merge, not racing it (AfterTargetsis OR, not AND — listing both anchors doesn't fix the ordering).bin/assembly overpublish/'s copy: the SDK's publish pipeline stages the primary assembly fromobj/(the pre-merge output) regardless of ordering, so without this the merged bytes still wouldn't reachpublish/.PublishOnBuild=trueforProjectType=WorkflowActivity, where nothing else provides that default. Kept the same default forPlugintoo, for symmetry and as a safeguard if the Microsoft package's own default ever changes — it's a no-op today sinceMicrosoft.PowerApps.MSBuild.Pluginalready sets it.Not verified
PdPackageprojects.EnsurePluginAssemblyDataXmlrequires a strong-name-signed assembly beyond this fix's scope; verified up to that point).Companion fix
A related fix is needed in
talxis/tools-devkit-templates(GenerateAssembly.csnet462 hardcoding) for a fully working Dataverse plugin + solution build.