Skip to content

Fix patching#796

Open
papeh wants to merge 4 commits intomainfrom
fix/patching
Open

Fix patching#796
papeh wants to merge 4 commits intomainfrom
fix/patching

Conversation

@papeh
Copy link
Copy Markdown
Contributor

@papeh papeh commented Mar 31, 2026

Summary

Download and copy DLLs during the build so that they are included in the installer.

Args.dll (needed for GeckofxHtmlToPdf) was not being copied during the build after .NET modernizations. It was also

Also

  • Include harvests in patch logs to make this type of omission easier to troubleshoot in the future.
  • Fix whitespace and build instructions in Setup-InstallerBuild.ps1
  • Miscellaneous cleanup
    • Fix whitespace and build instructions in Setup-InstallerBuild.ps1
    • Fix a garbled comment
    • Remove redundant -BuildTests flag (implied by -RunTests)
    • Ignore test residue

This change is Reviewable

papeh added 3 commits March 31, 2026 10:24
Args.dll (needed for GeckofxHtmlToPdf) was not being copied
during the build after .NET modernizations.

Also include harvests in patch logs to make this type of
omission easier to troubleshoot in the future.

Also fix whitespace and build instructions in
Setup-InstallerBuild.ps1
* Fix a garbled comment
* Remove redundant -BuildTests flag (implied by -RunTests)
* Ignore test residue
Of course downloaded files don't exist when the targets file
containing downloadDLLs is loaded.
@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 31, 2026

NUnit Tests

    1 files  ±0      1 suites  ±0   6m 4s ⏱️ +9s
4 074 tests ±0  4 003 ✅ ±0  71 💤 ±0  0 ❌ ±0 
4 083 runs  ±0  4 012 ✅ ±0  71 💤 ±0  0 ❌ ±0 

Results for commit 2eb77f5. ± Comparison against base commit 28e9888.

♻️ This comment has been updated with latest results.

CopyDlls was already being called by some other mysterious process.

This reverts the title change of commit
df33819.
@jasonleenaylor
Copy link
Copy Markdown
Contributor

Build/PackageRestore.targets line 454 at r1 (raw file):

			<ExistingPalasoFiles
				Include="@(PalasoFiles)"
				Condition="Exists('$(PalasoArtifactsDir)/%(Identity)')"

Yeah, this seems like AI slop that we missed.

@jasonleenaylor
Copy link
Copy Markdown
Contributor

Build/PackageRestore.targets line 560 at r1 (raw file):

			     SIL.BuildTasks is now a PackageReference in FwBuildTasks.csproj. -->
		</ItemGroup>
		<!-- REVIEW (Hasso) 2026.03: do we still need to copy NuGottenFiles explicitly here? -->

This bears further investigation.

Copy link
Copy Markdown
Contributor

@jasonleenaylor jasonleenaylor left a comment

Choose a reason for hiding this comment

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

:lgtm:

@jasonleenaylor reviewed 6 files and all commit messages, and made 1 comment.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on papeh).

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 this pull request may close these issues.

2 participants