PR Review: Add ARM64EC Support

Summary

This PR replaces Win32 (x86) platform configurations with ARM64EC across all Visual Studio project files and build scripts for the TemplateProject. ARM64EC (Emulation Compatible) allows ARM64 binaries to work with x64-only dependencies, which is valuable for Windows on ARM.

Code Quality & Best Practices

Strengths:

• Systematic approach: The changes comprehensively update all project configurations (AAX, VST2, VST3, CLAP, app) consistently
• Parallel architecture support: Maintains x64 alongside ARM64EC rather than replacing it, allowing multi-architecture builds
• Build script coordination: Updates to postbuild-win.bat, makedist-win.bat, and makezip-win.py properly reflect the architecture change

Concerns:

1. No backward compatibility for Win32/x86

   • The PR completely removes Win32 support without deprecation
   • Projects relying on 32-bit Windows builds will break
   • Recommendation: Consider whether Win32 should be maintained alongside ARM64EC and x64, or if this is an intentional breaking change that should be documented in release notes

2. Postbuild script signature change

   • The postbuild-win.bat parameter list changed significantly:
     • Old: VST2_32_PATH, VST2_64_PATH, VST3_32_PATH, etc.
     • New: VST2_ARM64EC_PATH, VST2_X64_PATH, VST3_ARM64EC_PATH, CLAP_X64_PATH, ICUDAT_PATH
   • Added CLAP_X64_PATH as separate parameter (CLAP was previously single-path)
   • Added ICUDAT_PATH parameter
   • Issue: This breaks compatibility with any existing projects not based on TemplateProject
   • Recommendation: Document this breaking change clearly

3. Missing documentation

   • No updates to CLAUDE.md or README explaining ARM64EC support
   • No guidance on when to use ARM64EC vs x64
   • No mention of system requirements (Windows 11 ARM64?)

Potential Bugs

1. AAX Library references (TemplateProject-aax.vcxproj)

   • The .lib file exclusion conditions were updated but I don't see corresponding ARM64EC AAX SDK libraries referenced
   • Lines still reference AAXLibrary.lib and AAXLibrary_x64.lib but no AAXLibrary_ARM64EC.lib
   • Question: Does the AAX SDK provide ARM64EC libraries? This may cause linker errors

2. Submodule commit change

   • The iPlug2 submodule was updated (84ceab31 → 8ee96b36)
   • Issue: The PR description doesn't mention what changes are in the submodule update
   • Recommendation: Document what framework changes are included and whether they're required for ARM64EC

3. vcxproj.user files included

   • Files like TemplateProject-app.vcxproj.user are included in the diff
   • These are typically user-specific and shouldn't be committed
   • Recommendation: Verify these should be in version control

Performance Considerations

• ARM64EC binaries may have slight performance overhead compared to native ARM64 due to emulation compatibility layer
• For audio plugins, native ARM64 would be preferable when all dependencies support it
• No performance-related code changes in this PR

Security Concerns

No security issues identified. The changes are purely build configuration updates.

Test Coverage

Major concern: No evidence of testing

The PR should demonstrate:

1. Successful builds of all plugin formats (VST2, VST3, AAX, CLAP, standalone) for ARM64EC
2. Testing on Windows ARM64 hardware (or VM)
3. Verification that plugins load in ARM64EC DAWs
4. Confirmation that postbuild and distribution scripts work correctly

Recommendation: Add a comment documenting the test results before merging.

Additional Recommendations

1. Branch naming: The branch name 'arm64ec' is clear but consider 'feature/arm64ec-support' for clarity

2. Commit message: Add a detailed commit message explaining:

   • Why ARM64EC vs native ARM64
   • What the breaking changes are
   • System requirements

3. Migration guide: Create documentation for existing projects on how to:

   • Migrate from Win32 to ARM64EC
   • Update their build scripts if they've customized them

4. CI/CD: Consider adding ARM64EC build targets to GitHub Actions (if not already present)

Questions for Author

1. Is the removal of Win32 support intentional? Should it be maintained?
2. Have you verified AAX SDK support for ARM64EC?
3. What changes are in the iPlug2 submodule update? Are they ARM64EC-specific?
4. Have you tested this on actual Windows ARM64 hardware?
5. Should common-win.props changes also be reviewed (file was listed but diff was truncated)?

Summary

This is a significant architectural change that modernizes the Windows build system. The implementation appears methodical and complete, but lacks documentation and testing evidence. The main risks are:

• Breaking existing Win32-dependent workflows
• Potential AAX library compatibility issues
• Undefined submodule changes

Verdict: Request changes - needs documentation, test results, and clarification on AAX library support before merging.

Review generated by Claude Code

PR Review: Add ARM64EC Support

I've reviewed PR #20 which adds ARM64EC (ARM64 Emulation Compatible) support to the iPlug2OOS template project. This is a significant enhancement that enables Windows on ARM compatibility.

Overall Assessment

Strengths:

• Comprehensive architectural update across all plugin formats (VST2, VST3, AAX, CLAP, standalone)
• Systematic replacement of Win32 with ARM64EC throughout the build system
• Consistent naming conventions and path variables
• Proper handling of platform-specific dependencies (icudtl.dat)

Detailed Findings

1. Code Quality & Best Practices

✅ Good:

• Consistent platform naming: All Win32 references systematically replaced with ARM64EC (common-win.props:36-49)
• Proper conditional logic in postbuild-win.bat:52-125 with platform-specific paths
• Updated makezip-win.py:64,73-74,81-86 to include ARM64EC binaries in archives

⚠️ Concerns:

• Redundant bundle creation calls in postbuild-win.bat:
  • Lines 106 and 112 both call CREATE_BUNDLE_SCRIPT for AAX
  • Lines 92 and 99 both call CREATE_BUNDLE_SCRIPT for VST3
  • The first call appears unnecessary since the bundle is created again before copying
• Batch parameter handling: The shift operations (postbuild-win.bat:14-31) are complex and error-prone. Consider using named parameters or a more maintainable approach.

2. Potential Bugs

🐛 Critical:

• Missing CLAP path in postbuild command: TemplateProject-win.props:27 adds CLAP_ARM64EC_PATH and CLAP_X64_PATH parameters, but these weren't in the original script. Need to verify the old script didn't rely on CLAP_PATH being position-dependent.

🐛 Medium:

• Case sensitivity: AAX bundle path uses "Arm64ec" (postbuild-win.bat:107) while VST3 uses "arm64ec-win" (postbuild-win.bat:93). Verify this matches the expected bundle structure conventions for each format.
• makedist-win.bat: Only builds specific targets (app, vst3, clap) on lines 71 and 74. AAX and VST2 are in comments but path variables are still updated - this may confuse future maintainers.

3. Performance Considerations

✅ Good:

• Parallel builds maintained with /m flag (makedist-win.bat:71,74)
• Conditional copying with exist checks reduces unnecessary operations
• MultiProcessorCompilation enabled (common-win.props:90)

💡 Suggestion:

• Consider adding ARM64EC-specific log file (makedist-win.bat:74) which would make troubleshooting easier. Currently both platforms append to different log files which is good.

4. Security Concerns

✅ Good:

• No credential exposure in the changes
• AAX code signing hooks properly commented out (makedist-win.bat:76-82)
• Proper use of quoted paths to handle spaces (postbuild-win.bat throughout)

⚠️ Minor:

• icudtl.dat file handling (postbuild-win.bat:53-60, 129-136) copies without validation. Consider adding checksum verification to ensure file integrity.

5. Test Coverage

❌ Major Gap:

• No CI/CD updates for ARM64EC builds. The build-native.yml workflow still only tests macOS and Windows (x64 implied)
• No pluginval testing for ARM64EC binaries
• makedist-win.bat:73-74 builds ARM64EC but the CI doesn't validate these builds

Recommendation: Update .github/workflows/build-native.yml to:

• Build ARM64EC configurations
• Test ARM64EC plugins (requires ARM64 runner or emulation)
• Include ARM64EC artifacts in release packages

6. Documentation & Maintainability

✅ Good:

• Clear echo statements in postbuild-win.bat showing all variables
• Comments explaining architecture changes
• Consistent variable naming convention

💡 Suggestions:

• Add comments explaining Win32→ARM64EC migration rationale
• Document icudtl.dat requirement for ARM64EC builds
• Consider updating CLAUDE.md to mention ARM64EC support

Architectural Concerns

1. vcxproj.user files: Changes to .vcxproj.user files (TemplateProject-app.vcxproj.user, TemplateProject-vst2.vcxproj.user, TemplateProject-vst3.vcxproj.user) are included. These are typically user-specific and shouldn't be in version control. Verify this is intentional for the template.

2. Breaking change: Projects duplicated from this template before this PR will need migration guidance. Consider providing a migration script or documentation.

3. Submodule update: iPlug2 submodule updated (changed SHA). Ensure this update includes necessary ARM64EC support in the framework itself.

Recommendations

High Priority:

1. Remove redundant CREATE_BUNDLE_SCRIPT calls in postbuild-win.bat
2. Add CI/CD support for ARM64EC builds
3. Verify CLAP path parameter changes don't break existing workflows
4. Validate bundle architecture folder naming (Arm64ec vs arm64ec-win)

Medium Priority:
5. Add migration documentation for existing projects
6. Consider refactoring postbuild-win.bat parameter handling for better maintainability
7. Add icudtl.dat integrity checking

Low Priority:
8. Update CLAUDE.md with ARM64EC support notes
9. Consider excluding .vcxproj.user files or documenting why they're needed in template

Verdict

This is a well-executed architectural change that brings important ARM64 support to Windows. The implementation is systematic and thorough. However, recommend addressing the CI/CD gap and redundant bundle creation calls before merging. The changes are otherwise ready with minor refinements.

Would you like me to help with any of these issues or create follow-up tasks?

@claude why does pluginval fail? can you fix that

Claude finished @olilarkin's task in 2m 12s —— View job

Pluginval Failure Analysis

Todo List:

• Check CI status and identify pluginval failures
• Download and analyze pluginval logs
• Identify root cause of the failure
• Attempted fix (blocked by permissions)

Root Cause

The pluginval failure occurs because the VST3 bundle now contains both architectures:

• TemplateProject.vst3/Contents/arm64ec-win/TemplateProject.vst3
• TemplateProject.vst3/Contents/x86_64-win/TemplateProject.vst3

When pluginval runs on x64 Windows (line 110 of .github/workflows/build-native.yml), it validates TemplateProject.vst3 at the bundle root. With multiple architectures present, it attempts to load the ARM64EC binary, which fails on x64 Windows.

Required Fix

The workflow needs to be updated to validate the architecture-specific binary instead of the bundle root:

Current (line 110):

pluginval.exe --skip-gui-tests --validate-in-process --output-dir "./bin" --validate ${{env.PROJECT_NAME}}.vst3

Should be:

pluginval.exe --skip-gui-tests --validate-in-process --output-dir "./bin" --validate ${{env.PROJECT_NAME}}.vst3/Contents/x86_64-win/${{env.PROJECT_NAME}}.vst3

Permission Issue

I attempted to make this change but cannot push it because:

! [remote rejected] HEAD -> arm64ec (refusing to allow a GitHub App to create or update workflow `.github/workflows/build-native.yml` without `workflows` permission)

GitHub Apps require explicit workflows permission to modify workflow files. You'll need to either:

1. Make the change manually: Edit .github/workflows/build-native.yml line 110 to point to the x64-specific binary path
2. Grant workflows permission: Update the GitHub App permissions (though this may not be desirable for security reasons)

Alternative Approach (if applicable)

If you want pluginval to test both architectures, you could add a second step that validates the ARM64EC binary using an ARM64 runner, but this would require Windows 11 ARM64 runners which may not be readily available in GitHub Actions.

--- | Branch: arm64ec

PR Review: Add ARM64EC Support

Summary

This PR adds ARM64EC (ARM64 Emulation Compatible) support to the iPlug2OOS build system, replacing 32-bit Windows builds. This is a forward-looking change as Windows on ARM gains adoption.

Code Quality & Best Practices

Strengths:

• Systematic approach: All Visual Studio solution and project files have been consistently updated
• Good separation of concerns: ARM64EC and x64 builds are kept separate with dedicated paths
• Build script improvements show attention to detail (e.g., handling icudtl.dat file copying)

Areas for Improvement:

1. postbuild-win.bat (Lines 52-125): The ARM64EC and x64 code blocks are nearly identical with substantial duplication. Consider refactoring into a function/subroutine:

   CALL :CopyBinaries "%PLATFORM%" ...
   GOTO :EOF
   
   :CopyBinaries
   REM Shared logic here

2. makezip-win.py (Line 85): The PDB file list is hardcoded. Consider using a loop or glob pattern to automatically include all PDFs matching the pattern, making it more maintainable:

   import glob
   pdb_files = glob.glob(projectpath + "\\build-win\\pdbs\\TemplateProject-*_*.pdb")

3. Missing Error Handling: The batch scripts don't check if required paths exist before attempting operations. Add validation:

   if not exist "%ICUDAT_PATH%" (
       echo ERROR: icudtl.dat not found
       exit /b 1
   )

Potential Bugs & Issues

1. Critical: pluginval Path Fix (.github/workflows/build-native.yml:110): Good catch fixing the VST3 validation path! However, this assumes the x86_64-win folder always exists. Consider adding a check or fallback.

2. postbuild-win.bat (Lines 78, 158): Duplicate conditional checks for .dll format with VST2 copying. The second check will never execute differently from the first. This appears to be a logic error - one should likely be checking a different format.

3. makedist-win.bat Parameter Mismatch: The postbuild-win.bat script now expects 18 parameters (including ICUDAT_PATH and extra CLAP paths), but I couldn't verify if makedist-win.bat was updated accordingly. Verify parameter passing is consistent.

4. makezip-win.py (Lines 62-71): File existence checks are done but errors aren't raised. Consider logging warnings or using assertions:

   if not os.path.exists(f):
       print(f"WARNING: File not found: {f}")

Performance Considerations

1. Parallel Builds: The .sln file configures both ARM64EC and x64 to build, which is good. However, consider documenting build times as ARM64EC compilation may be slower on x64 machines.

2. postbuild-win.bat: Multiple sequential copy operations. Consider using xcopy /Y for better performance with multiple files.

Security Concerns

1. Path Injection: The batch scripts use user-provided paths without sanitization. While this is typical for build scripts, document that these should only be run in trusted build environments.

2. AAX SDK Paths: The common-win.props hardcodes AAX SDK paths. Ensure these paths don't expose sensitive build system information in public repos.

Architecture & Design

1. Platform Naming Consistency: The codebase uses both "ARM64EC" and "Arm64ec" (e.g., line 107 in postbuild-win.bat uses Arm64ec for AAX). Standardize on one casing throughout.

2. Deprecation of Win32: Completely removing Win32 support is a breaking change. Consider:

   • Adding a deprecation notice in commit messages/changelog
   • Documenting the migration path for users still on 32-bit systems
   • Mentioning Windows 11 ARM as the primary target platform

3. common-win.props Complexity: This file has grown to 326 lines with many hardcoded paths. Consider:

   • Splitting into separate files (paths.props, defs.props, etc.)
   • Using more environment variable overrides for flexibility

Test Coverage

Missing:

1. No automated tests verify ARM64EC binaries work correctly
2. No CI/CD validation that ARM64EC builds actually run on ARM64 systems
3. The pluginval test only validates x64 builds (line 110 of workflow)

Recommendations:

1. Add ARM64EC-specific pluginval test if possible
2. Consider matrix builds in CI to test both architectures
3. Add smoke tests that verify plugin loads in ARM64EC environment

Additional Observations

1. Documentation: No updates to documentation files explaining ARM64EC support. Add to README or build instructions.

2. Submodule Update: The iPlug2 submodule was updated. Ensure that commit includes compatible ARM64EC changes.

3. Bundle Structure: The VST3 bundle now uses arm64ec-win folder (postbuild-win.bat:93). Verify this matches VST3 specification.

Recommendations Summary

Must Fix:

• Resolve duplicate .dll format checks in postbuild-win.bat (lines 78, 158)
• Verify makedist-win.bat parameter passing consistency
• Fix platform naming inconsistency (ARM64EC vs Arm64ec)

Should Fix:

• Add error handling to batch scripts
• Refactor duplicated code in postbuild-win.bat
• Update documentation

Nice to Have:

• Add ARM64EC test coverage
• Improve makezip-win.py with dynamic file discovery
• Split common-win.props into smaller files

Conclusion

This is a solid modernization effort with systematic changes across the build system. The main concerns are around code duplication, missing error handling, and test coverage. With the critical fixes applied, this will be a valuable addition for Windows on ARM support.