Skip to content

GHA CI: Use preinstalled vcpkg#15355

Merged
Chocobo1 merged 1 commit into
qbittorrent:masterfrom
Chocobo1:win
Aug 22, 2021
Merged

GHA CI: Use preinstalled vcpkg#15355
Chocobo1 merged 1 commit into
qbittorrent:masterfrom
Chocobo1:win

Conversation

@Chocobo1
Copy link
Copy Markdown
Member

@Chocobo1 Chocobo1 commented Aug 21, 2021

This will save maintenance work on the vcpkg version.

Also a few other improvements:

  • Add quotes to path
  • Sort command flags
  • Avoid switching shell, always use powershell (the default shell)

@Chocobo1 Chocobo1 added the CI Issues/PRs related to CI label Aug 21, 2021
@glassez
Copy link
Copy Markdown
Member

glassez commented Aug 21, 2021

@Chocobo1
It seemed to me that you were against it: #15342 (comment)

@Chocobo1
Copy link
Copy Markdown
Member Author

Chocobo1 commented Aug 21, 2021

It seemed to me that you were against it: #15342 (comment)

That was my outdated opinion. Later I discovered doNotUpdateVcpkg: true and done some testing, it seems the suggestion was actually viable (and the caching will still work as expected).

glassez
glassez previously approved these changes Aug 21, 2021
@xavier2k6
Copy link
Copy Markdown
Member

xavier2k6 commented Aug 21, 2021

There doesn't seem to be a need for this line at all below:

https://github.com/qbittorrent/qBittorrent/blob/ea5c23224c18dc9fe0ec97c65438ef88eb6dd1ad/.github/workflows/ci_windows.yaml#L23

Looking at logs:
It finds & uses the default commit in CI image as per https://github.com/actions/virtual-environments/blob/main/images/win/Windows2019-Readme.md#package-management

tool: C:\Program Files\Git\bin\git.exe
Fetching the Git commit id at 'C:\vcpkg' ...
6bc4362fb49e53f1fff7f51e4e27e1946755ecc6
vcpkg identified at commitId='6bc4362fb49e53f1fff7f51e4e27e1946755ecc6', adding it to the cache's key.

This will save maintenance work on the vcpkg version.

Also a few other improvements:
* Add quotes to path
* Sort command flags
* Avoid switching shell, always use powershell (the default shell)
@Chocobo1
Copy link
Copy Markdown
Member Author

@xavier2k6
Copy link
Copy Markdown
Member

xavier2k6 commented Aug 21, 2021

Would it also be advisable for consistency & less maintenance to use "latest" label for CI Images across all CI's?
https://github.com/actions/virtual-environments#available-environments

@Chocobo1
Copy link
Copy Markdown
Member Author

Would it also be advisable for consistency & less maintenance to use "latest" label for CI Images across all CI's?

No, IIRC at least on ubuntu, package names might change for different releases.

As for windows and macos you can submit a PR and discuss it, I'm not so eager to make such changes myself.

@Chocobo1 Chocobo1 merged commit 70573eb into qbittorrent:master Aug 22, 2021
@Chocobo1 Chocobo1 deleted the win branch August 22, 2021 04:29
Chocobo1 added a commit to Chocobo1/qBittorrent that referenced this pull request Aug 22, 2021
By having unified github workflows, the building cache would be utilized
more efficiently as the total cache size will be smaller (no more vcpkg
caching on macOS CI) and will stop thrashing the build cache (large
vcpkg cache evicts other smaller cache).

Relevant PRs:
qbittorrent#15321
qbittorrent#15340
qbittorrent#15342
qbittorrent#15355
Chocobo1 added a commit that referenced this pull request Aug 23, 2021
By having unified github workflows, the building cache would be utilized
more efficiently as the total cache size will be smaller (no more vcpkg
caching on macOS CI) and will stop thrashing the build cache (large
vcpkg cache evicts other smaller cache).

Relevant PRs:
#15321
#15340
#15342
#15355
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI Issues/PRs related to CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants