Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: vcpkg binaries were not being cached on Mac #8571

Merged
merged 1 commit into from Jan 14, 2021
Merged

Conversation

@orudge
Copy link
Contributor

@orudge orudge commented Jan 14, 2021

Motivation / Problem

vcpkg was being cloned into the /tmp directory on Mac, and this was somehow breaking the caching.

Description

We now re-clone vcpkg into the standard directory. Once the Mac build agents are updated to a more recent version of vcpkg we can drop this step.

cd /tmp
git clone https://github.com/Microsoft/vcpkg
cd /usr/local/share/vcpkg
ls -A1 | xargs rm -rf

This comment has been minimized.

@TrueBrain

TrueBrain Jan 14, 2021
Member

why not just go into /usr/local/share and do rm -rf vcpkg ? After that you can clone it again with vcpkg instead of .?

This comment has been minimized.

@orudge

orudge Jan 14, 2021
Author Contributor

Because I think that's what I did originally (when first doing the arm64 branch) and the permissions on /usr/local/share didn't allow me to. I suppose I could do a sudo rm -rf etc then recreate it, but this way ultimately worked out a bit simpler and avoided sudo.

https://github.com/actions/virtual-environments/blob/main/images/macos/provision/core/vcpkg.sh shows the chmod in opreation.

That said, I see now the repository there just had a shallow clone, so perhaps a git fetch --unshallow is all I need to do instead...

This comment has been minimized.

@orudge

orudge Jan 14, 2021
Author Contributor

Yes, replaced now with a git fetch which looks to be working.

@orudge orudge force-pushed the orudge:vcpkg-fix branch from 493f02a to bed734e Jan 14, 2021
- name: Prepare vcpkg (with cache)
uses: lukka/run-vcpkg@v6
with:
vcpkgDirectory: '/tmp/vcpkg'
vcpkgDirectory: '/usr/local/share/vcpkg'
doNotUpdateVcpkg: false

This comment has been minimized.

@TrueBrain

TrueBrain Jan 14, 2021
Member

Can this also be put back on true in this case?

This comment has been minimized.

@orudge

orudge Jan 14, 2021
Author Contributor

We do need it to update to the git changeset ID we specified (the git fetch won't check out the changeset we need of course), so I think it still needs to stay as it is. It's caching fine in my test environment.

This comment has been minimized.

@TrueBrain

TrueBrain Jan 14, 2021
Member

So you only fetch, not checkout, fair. In that case would you mind extending the comment above to indicate this should also be set back to true once the base image is updated?
I only mention it, as this is now different from the Windows target, and that is going to confuse is in the future :D

This comment has been minimized.

@orudge

orudge Jan 14, 2021
Author Contributor

Comment updated.

This comment has been minimized.

@LordAro

LordAro Jan 14, 2021
Member

Is the same not needed for the ci-build?
(or, what is an updated vcpkg needed for?)

This comment has been minimized.

@orudge

orudge Jan 14, 2021
Author Contributor

No, the updated vcpkg is only needed for arm64 builds on x86_64 (or building x86_64 on arm64), and we don't currently build arm64 on the CI.

@orudge orudge force-pushed the orudge:vcpkg-fix branch from bed734e to d97ab66 Jan 14, 2021
@orudge orudge merged commit bc8f347 into OpenTTD:master Jan 14, 2021
8 checks passed
8 checks passed
Emscripten
Details
Commit checker
Details
Check preview needs update Check preview needs update
Details
Linux (clang, clang++)
Details
Linux (gcc, g++)
Details
Mac OS (x64, x86_64)
Details
Windows (x86)
Details
Windows (x64)
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants