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

Additional fixes to Appveyor build #46220

Closed
wants to merge 22 commits into from

Conversation

NorseFTX
Copy link
Contributor

@NorseFTX NorseFTX commented Dec 21, 2020

Summary

SUMMARY: Bugfixes "Testing fixes to Appveyor build; EDIT: Solution outlined in solution section below"

Purpose of change

The current Appveyor build fails when unable to find the vcpkg files in the currently existing cache.
Disclaimer: This change will not fix the Appveyor build permanently, but is meant to outline the process necessary to update appveyor.yml to make sure the build compiles. This requires three steps: Clearing the cache, repopulating the cache, and then building. This probably only needs to be run when vcpkg packages are updated substantially, and doesn't have to be run constantly.

Describe the solution

Working off the following code (included with the changes from this commit):
Appveyor.yml

version: '{branch}.{build}'
image: Visual Studio 2019
configuration: Release
platform: x64
environment:
 APPVEYOR_SAVE_CACHE_ON_ERROR: true
shallow_clone: true
clone_folder: C:\Projects\Cataclysm-DDA
cache:
  - c:\tools\vcpkg\installed\
  - c:\Users\appveyor\AppData\Local\vcpkg\
install:
 - cmd: vcpkg install yasm-tool:x86-windows
 - cmd: vcpkg --triplet x64-windows-static install sdl2 sdl2-image sdl2-mixer[dynamic-load,libflac,mpg123,libmodplug,libvorbis] sdl2-ttf gettext
build:
  project: /msvc-full-features/Cataclysm-vcpkg-static.sln
  parallel: true
  verbosity: minimal
test_script:
  - cmd: Cataclysm-test-vcpkg-static-Release-x64.exe --rng-seed time --min-duration 0.2

I. Clear existing cache - Time ~20 sec

  1. Add -> appveyor.yml dependency cache lines, to invalidate cache files:
cache:
  - c:\tools\vcpkg\installed\ -> appveyor.yml
  - c:\Users\appveyor\AppData\Local\vcpkg\ -> appveyor.yml
  1. Comment out all install/build/test lines:
# install:
 # - cmd: vcpkg install yasm-tool:x86-windows
 # - cmd: vcpkg --triplet x64-windows-static install sdl2 sdl2-image sdl2-mixer[dynamic-load,libflac,mpg123,libmodplug,libvorbis] sdl2-ttf gettext
# build:
  # project: /msvc-full-features/Cataclysm-vcpkg-static.sln
  # parallel: true
  # verbosity: minimal
# test_script:
  # - cmd: Cataclysm-test-vcpkg-static-Release-x64.exe --rng-seed time --min-duration 0.2
  1. Push change to allow Appveyor to compile. Should result in a build error, and the cache being cleared. By default, cache cannot be modified by a pull request, as it should be (allowing pull requests to modify the cache is dangerous, probably). As a result, the commit will have to be made to the main master repository for the cache to clear successfully. Follows documentation (https://www.appveyor.com/docs/build-cache/).
    image
    From: https://ci.appveyor.com/project/NorseFTX/cataclysm-dda/builds/36952667

II. Force cache update - Time ~20+ min

  1. Remove -> appveyor.yml dependency from cache lines
cache:
  - c:\tools\vcpkg\installed\
  - c:\Users\appveyor\AppData\Local\vcpkg\
  1. Uncomment test lines
install:
 - cmd: vcpkg install yasm-tool:x86-windows
 - cmd: vcpkg --triplet x64-windows-static install sdl2 sdl2-image sdl2-mixer[dynamic-load,libflac,mpg123,libmodplug,libvorbis] sdl2-ttf gettext
  1. Push change to allow Appveyor to compile. Should result in the build failing, but the cache updating successfully.
    image
    Example: https://ci.appveyor.com/project/NorseFTX/cataclysm-dda/builds/36952762

III. Enable Build - Time ~60+ min

  1. Uncomment build/test lines
build:
  project: /msvc-full-features/Cataclysm-vcpkg-static.sln
  parallel: true
  verbosity: minimal
test_script:
  - cmd: Cataclysm-test-vcpkg-static-Release-x64.exe --rng-seed time --min-duration 0.2
  1. Push change. Appveyor should now compile correctly, and recognize the cached files. Note that parts I and II only need to be performed if the vcpkg files are updated and enough parts of the cache are invalidated that the package installation process ends up timing out the Appveyor build in the future.
    image
    Example: https://ci.appveyor.com/project/NorseFTX/cataclysm-dda/builds/36952961

Unfortunately, from the appveyor build cache documentation, there is no (?) way to push a cache update if a build times out. It is only possible upon error or upon successful build (neither of which a time out counts as).

Describe alternatives you've considered

This is a log of the troubleshooting process, for your perusal if you are interested.

Issues:

  1. Current appveyor cache probably needs updating
  2. Problem: Appveyor by default will not update the cache unless the build finishes and succeeds
  3. Problem 2: CDDA is so dang big that Appveyor times out before it even finishes compiling, so the build is never finished, and the cache never updates; the fact that the cache isn't updating means it has to install everything over again, which ends up making the build time out again

Solution:

  • To update the cache, enable the cache to save even when there is a build error by adding the lines:
environment:
  APPVEYOR_SAVE_CACHE_ON_ERROR: true
  • New Problem: A build timeout doesn't count as a build success OR a build error, so the cache doesn't update

Solution 2:

  • Force Appveyor to crash prematurely to update the cache. The easiest way is to comment out ('#') all the build lines.
  • Run it again after the initial crash updates the cache and the thing compiles without reaching the build time out

Log of successful compile on my fork:
https://ci.appveyor.com/project/NorseFTX/cataclysm-dda

Appveyor Build Cache documentation:
https://www.appveyor.com/docs/build-cache/
Appveyor Environmental Variables list:
https://www.appveyor.com/docs/environment-variables/

Testing

This was tested on a fork of the Cataclysm-DDA repository, with the build history visible here:
https://ci.appveyor.com/project/NorseFTX/cataclysm-dda/history

Additional context

This would probably not be an issue if Appveyor allowed more than 2 hours for compiling before a timeout occurs.

@NorseFTX NorseFTX closed this Dec 21, 2020
@ZhilkinSerg
Copy link
Contributor

That won't do anything.

First you need to run git pull in vcpkg installation folder, but then again - there aren't any packages installed to be upgraded.

@NorseFTX
Copy link
Contributor Author

NorseFTX commented Dec 21, 2020

Yeah I tried that with the second and realized wasn't in the right directory; I'll probably test this in a clone instead of on the official repository xP The packages do seem up to date so that might not be the issue.

@ZhilkinSerg
Copy link
Contributor

You can create a branch and build it on Appveyor without creating a PR if you add your repo to Appveyor.

@ZhilkinSerg
Copy link
Contributor

Also see microsoft/vcpkg#15087 for yasm-tool issue details.

@NorseFTX
Copy link
Contributor Author

Was able to set up Appveyor on my own fork and test things--the compiling appears to work on my end, so I'll go ahead and reopen with the new changes. I have a feeling caching may be one of the issues the current build is running into (?); will have to check to see.

@NorseFTX NorseFTX reopened this Dec 21, 2020
@NorseFTX NorseFTX closed this Dec 21, 2020
@NorseFTX
Copy link
Contributor Author

A solution is available for the Appveyor build error outlined (requires three separate commits each time the cache needs to be updated, unfortunately). Depending on how important the appveyor functionality is, this can be done regularly when it fails to build, or possibly not at all if its functionality isn't that important.

@NorseFTX NorseFTX reopened this Dec 22, 2020
@ZhilkinSerg
Copy link
Contributor

If it is only about cache - it is not something that requires in-repo changes. Build cache can be cleared without creating pull requests by other means - https://www.appveyor.com/docs/build-cache/#cleaning-up-cache.

@mlangsdorf mlangsdorf added Code: Build Issues regarding different builds and build environments Code: Tests Measurement, self-control, statistics, balancing. Code: Tooling Tooling that is not part of the main game but is part of the repo. labels Dec 22, 2020
@NorseFTX
Copy link
Contributor Author

NorseFTX commented Dec 22, 2020

Yup, you can probably skip the first step of cleaning the cache by calling the API to clear the cache as shown in the link you posted instead of forcing a new build. You'll still need to repopulate the cache without the build timing out though, so steps 2 and 3 might still be necessary.
I haven't tested the API myself but the steps above have been tested and work, and accomplish the same thing. With the above method, step 1 (cache clearing) is probably the least time consuming, taking an average of 20 seconds. The rest of the steps are the ones that usually take more time.

@ZhilkinSerg
Copy link
Contributor

Let's just switch to x86 for the start - #46352.

@ZhilkinSerg ZhilkinSerg added the OS: Windows Issues related to Windows operating system label Dec 27, 2020
@ZhilkinSerg
Copy link
Contributor

Let's close it for now (until #46352 works for us).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code: Build Issues regarding different builds and build environments Code: Tests Measurement, self-control, statistics, balancing. Code: Tooling Tooling that is not part of the main game but is part of the repo. OS: Windows Issues related to Windows operating system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants