Skip to content

Conversation

@3dJan
Copy link
Owner

@3dJan 3dJan commented Nov 14, 2025

No description provided.

Copilot AI review requested due to automatic review settings November 14, 2025 10:22
@3dJan 3dJan merged commit 1273687 into develop Nov 14, 2025
4 checks passed
Copy link

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

This PR implements vcpkg binary caching using GitHub Packages and adds buildcache support to improve build performance in CI workflows.

Key changes:

  • Configures vcpkg to use GitHub Packages NuGet feed for binary caching with read/write permissions
  • Adds buildcache installation and caching for both Linux and Windows platforms
  • Sets up NuGet authentication to enable package uploads and downloads

Comment on lines +141 to +148
mono `${{ github.workspace }}/vcpkg/vcpkg fetch nuget | tail -n 1` \
sources add \
-Source "https://nuget.pkg.github.com/${{ github.repository_owner }}/index.json" \
-StorePasswordInClearText \
-Name GitHubPackages \
-UserName "${{ github.repository_owner }}" \
-Password "${{ secrets.GITHUB_TOKEN }}"
mono `${{ github.workspace }}/vcpkg/vcpkg fetch nuget | tail -n 1` \
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

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

The command substitution syntax is incorrect for bash. Use $(...) instead of backticks when the substitution contains ${{ }} GitHub Actions expressions. The backticks will cause shell parsing issues. Change to: mono $(vcpkg/vcpkg fetch nuget | tail -n 1)

Suggested change
mono `${{ github.workspace }}/vcpkg/vcpkg fetch nuget | tail -n 1` \
sources add \
-Source "https://nuget.pkg.github.com/${{ github.repository_owner }}/index.json" \
-StorePasswordInClearText \
-Name GitHubPackages \
-UserName "${{ github.repository_owner }}" \
-Password "${{ secrets.GITHUB_TOKEN }}"
mono `${{ github.workspace }}/vcpkg/vcpkg fetch nuget | tail -n 1` \
mono $(${{ github.workspace }}/vcpkg/vcpkg fetch nuget | tail -n 1) \
sources add \
-Source "https://nuget.pkg.github.com/${{ github.repository_owner }}/index.json" \
-StorePasswordInClearText \
-Name GitHubPackages \
-UserName "${{ github.repository_owner }}" \
-Password "${{ secrets.GITHUB_TOKEN }}"
mono $(${{ github.workspace }}/vcpkg/vcpkg fetch nuget | tail -n 1) \

Copilot uses AI. Check for mistakes.
Comment on lines +141 to +148
mono `${{ github.workspace }}/vcpkg/vcpkg fetch nuget | tail -n 1` \
sources add \
-Source "https://nuget.pkg.github.com/${{ github.repository_owner }}/index.json" \
-StorePasswordInClearText \
-Name GitHubPackages \
-UserName "${{ github.repository_owner }}" \
-Password "${{ secrets.GITHUB_TOKEN }}"
mono `${{ github.workspace }}/vcpkg/vcpkg fetch nuget | tail -n 1` \
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

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

Same command substitution syntax issue as above. Use $(...) instead of backticks. Change to: mono $(vcpkg/vcpkg fetch nuget | tail -n 1)

Suggested change
mono `${{ github.workspace }}/vcpkg/vcpkg fetch nuget | tail -n 1` \
sources add \
-Source "https://nuget.pkg.github.com/${{ github.repository_owner }}/index.json" \
-StorePasswordInClearText \
-Name GitHubPackages \
-UserName "${{ github.repository_owner }}" \
-Password "${{ secrets.GITHUB_TOKEN }}"
mono `${{ github.workspace }}/vcpkg/vcpkg fetch nuget | tail -n 1` \
mono $(${{ github.workspace }}/vcpkg/vcpkg fetch nuget | tail -n 1) \
sources add \
-Source "https://nuget.pkg.github.com/${{ github.repository_owner }}/index.json" \
-StorePasswordInClearText \
-Name GitHubPackages \
-UserName "${{ github.repository_owner }}" \
-Password "${{ secrets.GITHUB_TOKEN }}"
mono $(${{ github.workspace }}/vcpkg/vcpkg fetch nuget | tail -n 1) \

Copilot uses AI. Check for mistakes.
Comment on lines +156 to +163
& "$(${{ github.workspace }}/vcpkg/vcpkg fetch nuget)" `
sources add `
-Source "https://nuget.pkg.github.com/${{ github.repository_owner }}/index.json" `
-StorePasswordInClearText `
-Name GitHubPackages `
-UserName "${{ github.repository_owner }}" `
-Password "${{ secrets.GITHUB_TOKEN }}"
& "$(${{ github.workspace }}/vcpkg/vcpkg fetch nuget)" `
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

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

The PowerShell syntax is incorrect. The $(...) should wrap the entire command execution, not be used to concatenate strings with GitHub Actions expressions. Change to: & \"$(vcpkg/vcpkg fetch nuget)\" or use proper string interpolation.

Suggested change
& "$(${{ github.workspace }}/vcpkg/vcpkg fetch nuget)" `
sources add `
-Source "https://nuget.pkg.github.com/${{ github.repository_owner }}/index.json" `
-StorePasswordInClearText `
-Name GitHubPackages `
-UserName "${{ github.repository_owner }}" `
-Password "${{ secrets.GITHUB_TOKEN }}"
& "$(${{ github.workspace }}/vcpkg/vcpkg fetch nuget)" `
& "$(${{ github.workspace }}\vcpkg\vcpkg fetch nuget)" `
sources add `
-Source "https://nuget.pkg.github.com/${{ github.repository_owner }}/index.json" `
-StorePasswordInClearText `
-Name GitHubPackages `
-UserName "${{ github.repository_owner }}" `
-Password "${{ secrets.GITHUB_TOKEN }}"
& "$(${{ github.workspace }}\vcpkg\vcpkg fetch nuget)" `

Copilot uses AI. Check for mistakes.
Comment on lines +156 to +163
& "$(${{ github.workspace }}/vcpkg/vcpkg fetch nuget)" `
sources add `
-Source "https://nuget.pkg.github.com/${{ github.repository_owner }}/index.json" `
-StorePasswordInClearText `
-Name GitHubPackages `
-UserName "${{ github.repository_owner }}" `
-Password "${{ secrets.GITHUB_TOKEN }}"
& "$(${{ github.workspace }}/vcpkg/vcpkg fetch nuget)" `
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

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

Same PowerShell syntax issue as above. The $(...) should wrap the entire command execution. Change to: & \"$(vcpkg/vcpkg fetch nuget)\" or use proper string interpolation.

Suggested change
& "$(${{ github.workspace }}/vcpkg/vcpkg fetch nuget)" `
sources add `
-Source "https://nuget.pkg.github.com/${{ github.repository_owner }}/index.json" `
-StorePasswordInClearText `
-Name GitHubPackages `
-UserName "${{ github.repository_owner }}" `
-Password "${{ secrets.GITHUB_TOKEN }}"
& "$(${{ github.workspace }}/vcpkg/vcpkg fetch nuget)" `
& "$(& '${{ github.workspace }}/vcpkg/vcpkg fetch nuget')" `
sources add `
-Source "https://nuget.pkg.github.com/${{ github.repository_owner }}/index.json" `
-StorePasswordInClearText `
-Name GitHubPackages `
-UserName "${{ github.repository_owner }}" `
-Password "${{ secrets.GITHUB_TOKEN }}"
& "$(& '${{ github.workspace }}/vcpkg/vcpkg fetch nuget')" `

Copilot uses AI. Check for mistakes.
Comment on lines +116 to +117
Copy-Item -Path "buildcache\buildcache\bin\buildcache.exe" -Destination "C:\Windows\System32\buildcache.exe" -Force
buildcache --version
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

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

Copying to C:\Windows\System32\ requires elevated permissions and may fail in GitHub Actions runners. Consider using a location in the PATH that doesn't require admin rights, such as adding the buildcache directory to $env:PATH or using a user-accessible directory.

Suggested change
Copy-Item -Path "buildcache\buildcache\bin\buildcache.exe" -Destination "C:\Windows\System32\buildcache.exe" -Force
buildcache --version
echo "$env:GITHUB_WORKSPACE\buildcache\buildcache\bin" | Out-File -Append -FilePath $env:GITHUB_PATH
buildcache\buildcache\bin\buildcache.exe --version

Copilot uses AI. Check for mistakes.
Comment on lines +124 to +126
key: buildcache-${{ matrix.os }}-${{ matrix.c_compiler }}-${{ github.sha }}
restore-keys: |
buildcache-${{ matrix.os }}-${{ matrix.c_compiler }}-
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

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

The cache key includes github.sha which means a new cache entry is created for every commit, preventing effective cache reuse across commits. Consider using a more stable key like the hash of dependency files or remove github.sha to allow cache hits across commits: key: buildcache-${{ matrix.os }}-${{ matrix.c_compiler }}-${{ hashFiles('**/vcpkg.json') }}

Copilot uses AI. Check for mistakes.
Comment on lines +133 to +135
key: buildcache-${{ matrix.os }}-${{ matrix.c_compiler }}-${{ github.sha }}
restore-keys: |
buildcache-${{ matrix.os }}-${{ matrix.c_compiler }}-
Copy link

Copilot AI Nov 14, 2025

Choose a reason for hiding this comment

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

Same issue as the Linux buildcache configuration. The cache key includes github.sha which creates a new entry for every commit. Use a more stable key to improve cache hit rate: key: buildcache-${{ matrix.os }}-${{ matrix.c_compiler }}-${{ hashFiles('**/vcpkg.json') }}

Copilot uses AI. Check for mistakes.
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