lukka / run-vcpkg

The GitHub Action to setup vcpkg for your C++ based projects. Stores built ports using Binary Caching backed onto GH Cache.
MIT License
195 stars 26 forks source link

respect VCPKG_FORCE_SYSTEM_BINARIES #217

Closed lionkor closed 9 months ago

lionkor commented 10 months ago

When VCPKG_FORCE_SYSTEM_BINARIES is set, run-vcpkg will try to build ports for both x64 and arm64. The former will fail due to incompatible compiler flags.

When VCPKG_FORCE_SYSTEM_BINARIES is set, the action should assume a different default triplet than x64, maybe the one of the platform (like vcpkg does by default).

lukka commented 10 months ago

@lionkor to make this request actionable I think I need your help:

Link (or attached log to this issue) to existing failing workflows would help.

lionkor commented 10 months ago

Sure!

To reproduce:

  1. Make a GitHub Actions workflow with run-vcpkg in a repo with vcpkg.json (manifest mode, so with runVcpkgInstall: true) with some dependency like boost-system.
  2. Set VCPKG_FORCE_SYSTEM_BINARIES: 1 in the workflow's ENV (this ensures vcpkg will build for ARM64, on ARM64).
  3. Run the GitHub Actions workflow in an ARM64 runner.
  4. Observe build failure due to run-vcpkg building for some x64 triplet instead of just the arm64 triplet.

The problem:

VCPKG_FORCE_SYSTEM_BINARIES means that vcpkg should ONLY build for the target triplet, not some cross-compilation triplet, in this case. Instead, it builds for another triplet, with the current system's compiler, which fails if the specific package uses any kind of special flags on another triplet (like x64's -m64 flag).

The observed behavior:

Build failure on arm64.

Logs:

https://pastebin.com/rvGhAdF0

The paste includes ONLY the run-vcpkg invokation, for your convenience, because the rest of the log is huge. You can see the VCPKG_FORCE_SYSTEM_BINARIES in the env for that run.

Please observe line 345 in this log for the incorrect detection of x64 as described above. Observe also line 786 where the arm64 gcc compiler obviously fails to build with x64 flags (because it does not support those flags, because they're specific to x64 afaik).

Expected behavior:

When running vcpkg in a bare-metal arm64 machine, with ONLY the VCPKG_FORCE_SYSTEM_BINARIES=1 set, vcpkg in manifest mode will correctly only build the arm64 builds as you can see in this log: https://pastebin.com/mT5V8ZNP.

So, to clarify: The expected behavior in this case is that the x64 triplet is not built when vcpkg install is invoked by run-vcpkg on arm64 with VCPKG_FORCE_SYSTEM_BINARIES set. vcpkg handles this properly, so it must be a run-vcpkg issue, as far as I could debug it.

lionkor commented 10 months ago

Here's a link to the actual workflow file, before hotfixing it by setting the triplet (which should NOT be needed since the run-vcpkg action runs on arm64 itself and vcpkg knows it's arm64: https://github.com/BeamMP/BeamMP-Server/actions/runs/7366964699/workflow (specifically only the arm64 workflows even call run-vcpkg, so you'll have to look at the debian-11-arm64-build job)

lukka commented 10 months ago

@lionkor thank you for a throughout explanation of the problem! It is indeed a run-vcpkg issue, the getDefaultTriplet needs to be augmented to be able to tell whether the default vcpkg triplet is for Intel or ARM based CPUs. The RUNNER_ARCH environment variable could be used to detect ARM vs Intel platforms, and if it is not set, the fallback would be os.arch(), which is not perfect as it reports the target platform of the nodejs executable, but it is good most of the times.

lionkor commented 10 months ago

Wonderful, thank you! Let me know if you need anything else, I have runners and arm64 machines to test with.

lukka commented 10 months ago

@lionkor you may try the run-vcpkg@dev/arm-triplet-fix which contains the proposed fix. Let me know if this is working for you as well!

lionkor commented 10 months ago

Thank you very much, looks sane to me. Will test when I get a chance & get back to you

lukka commented 9 months ago

@lionkor I proceeded forward and released a new version of run-vcpkg@v11.5 with the fix merged in. Let me know if it works for you, thank you.