Skip to content

[build] Replace Step_InstallGNUBinutils with MSBuild targets#11332

Open
jonathanpeppers wants to merge 3 commits into
mainfrom
dev/peppers/xaprepare-binutils
Open

[build] Replace Step_InstallGNUBinutils with MSBuild targets#11332
jonathanpeppers wants to merge 3 commits into
mainfrom
dev/peppers/xaprepare-binutils

Conversation

@jonathanpeppers
Copy link
Copy Markdown
Member

Migrate the binutils installation from xaprepare to a standard MSBuild Microsoft.Build.NoTargets project (src/binutils), as part of the ongoing migration away from xaprepare.

The new project downloads the toolchain archive, extracts it with 7-zip, and copies binutils to the install directories for both the host OS and Windows (including PDB symbol archiving).

@jonathanpeppers
Copy link
Copy Markdown
Member Author

/review

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 12, 2026

Android PR Reviewer completed successfully!

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

✅ LGTM — Clean migration from xaprepare to MSBuild targets

Summary: This PR replaces the C# Step_InstallGNUBinutils xaprepare step with a standard Microsoft.Build.NoTargets MSBuild project (src/binutils), following the established pattern used by src/aapt2 and src/bundletool. The migration is well-executed.

What's good:

  • ✅ SHA256 hash verification on the downloaded archive — an improvement over the old code which had none
  • ✅ Proper Inputs/Outputs on download and extract targets for incremental builds
  • SkipUnchangedFiles on Copy tasks minimizes redundant I/O
  • ✅ Correct handling of host vs Windows binutils install directories, matching old behavior
  • ✅ PDB+EXE symbol archiving logic is simplified with wildcard globs while preserving correctness
  • ✅ Clean removal of 5 old Step_InstallGNUBinutils files and the scenario registration

Issue counts: 0 ❌ | 1 ⚠️ | 2 💡

⚠️ One concern: The binutils version/URL/path configuration in Configurables.cs is now dead code (neither Step_InstallGNUBinutils nor Step_Get_Windows_Binutils is used), creating a dual source of truth with the new XABinutilsVersion/XABinutilsHash in Configuration.props. Worth cleaning up to prevent version drift.

CI status: All checks passing (dotnet-android ✅, license/cla ✅).

Generated by Android PR Reviewer for issue #11332 · ● 7.8M

Comment thread Configuration.props
Comment thread src/binutils/binutils.targets Outdated
Comment thread src/binutils/binutils.targets
jonathanpeppers and others added 2 commits May 13, 2026 09:12
Migrate the binutils installation from xaprepare to a standard
MSBuild `Microsoft.Build.NoTargets` project (`src/binutils`),
as part of the ongoing migration away from xaprepare.

The new project downloads the toolchain archive, extracts it with
7-zip, and copies binutils to the install directories for both the
host OS and Windows (including PDB symbol archiving).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove stale binutils entries from Configurables.cs (BinutilsVersion,
BinutilsArchive, WindowsBinutilsInstallDir, HostBinutilsInstallDir,
BinutilsCacheDir) and delete the unused Step_Get_Windows_Binutils.cs.

Fix MSBuild property function spacing to match repo conventions:
ToLowerInvariant() without a space before the parentheses.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jonathanpeppers jonathanpeppers force-pushed the dev/peppers/xaprepare-binutils branch from ad97e61 to 71b5134 Compare May 13, 2026 14:13
@jonathanpeppers jonathanpeppers marked this pull request as ready for review May 13, 2026 16:34
Copilot AI review requested due to automatic review settings May 13, 2026 16:34
@jonathanpeppers jonathanpeppers added the ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable). label May 13, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Migrates GNU binutils installation from the legacy xaprepare C# step (Step_InstallGNUBinutils and Step_Get_Windows_Binutils) to a new Microsoft.Build.NoTargets MSBuild project at src/binutils/. The new project downloads the prebuilt toolchain .7z archive, validates its SHA-256, extracts it via 7za, and installs the host ($(HostOS)\binutils) and Windows (binutils) layouts, including PDB symbol staging. This continues the ongoing migration away from xaprepare toward standard MSBuild projects.

Changes:

  • Adds src/binutils/binutils.csproj + binutils.targets, registers it in the solution, and chains it via a ProjectReference from Xamarin.Android.Build.Tasks; introduces XABinutilsVersion/XABinutilsHash in Configuration.props.
  • Removes Step_InstallGNUBinutils (and its OS-specific partials), Step_Get_Windows_Binutils, and the related BinutilsVersion/BinutilsArchive/*BinutilsInstallDir/BinutilsCacheDir configurables from xaprepare; drops the step from Scenario_Standard.
  • Uses 7-Zip.CommandLine (PackageReference, GeneratePathProperty) on Windows and 7za from PATH on Unix to extract the archive.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
Xamarin.Android.sln Registers the new binutils project in the solution.
src/Xamarin.Android.Build.Tasks/Xamarin.Android.Build.Tasks.csproj Adds non-output ProjectReference so binutils install runs as part of the tasks build.
src/binutils/binutils.csproj New Microsoft.Build.NoTargets project that pulls in 7-Zip.CommandLine and imports the targets.
src/binutils/binutils.targets New download/hash-verify/extract/install/clean targets for host and Windows binutils.
Configuration.props Adds XABinutilsVersion and XABinutilsHash properties.
build-tools/xaprepare/.../Scenario_Standard.cs Removes Step_InstallGNUBinutils from the standard scenario.
build-tools/xaprepare/.../ConfigAndData/Configurables.cs Removes BinutilsVersion, BinutilsArchive, *BinutilsInstallDir, BinutilsCacheDir.
build-tools/xaprepare/.../Steps/Step_InstallGNUBinutils*.cs Deletes the now-unused step and its OS partials.
build-tools/xaprepare/.../Steps/Step_Get_Windows_Binutils.cs Deletes the legacy custom ZIP-range download/extract path for Windows binutils.
build-tools/xaprepare/xaprepare/xaprepare.csproj Removes the orphaned Step_Get_Windows_Binutils.cs Compile item for non-Windows.

Comment thread src/binutils/binutils.targets Outdated
PDBs should only go to windows-toolchain-pdb/ for symbol archiving,
not into the binutils\bin\ directory. This matches the previous
xaprepare behavior and avoids duplicating ~1.3GB of symbol files.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-review This PR is ready to review/merge, I think any CI failures are just flaky (ignorable).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants