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

CI fixes and updates #1291

Merged
merged 9 commits into from
May 30, 2021
Merged

CI fixes and updates #1291

merged 9 commits into from
May 30, 2021

Conversation

ericoporto
Copy link
Member

@ericoporto ericoporto commented May 23, 2021

our Windows Dockerfile no longer builds, since the end of April, Visual Studio 2015 C++ Build Tools are no longer available through the web installer, and downloading the new offline installer for it from VS dowloads requires password, so to avoid modifying the build too much, I am using a docker image I pushed on dockerhub months ago that contains exactly it. I tried to pull from an alternate download on archive.org but it took just too much time to download it.

  • this PR makes the Windows Dockerfile build again
  • it removes Directx from the image and as dependency in the CMake scripts, since it's no longer required. The Directx installer we used is also no longer available on MS website.
  • additionally, updates SDL_sound to latest commit available (all builds). (fixes SDL2: particular OGG clip does not play #1275)
  • Moves from Xiph libraries to GitHub Xiph mirror because of TLS incompatibilities with Debian Jessie's curl (see Provide package that can be built with ./configure xiph/theora#16).
  • removes wrongly placed environment variables in our Editor build in the CI.
  • fixes win bsdtar bug that kept failing when packaging the editor for release

please wait cirrus-ci build this and confirm this PR works.

-Visual Studio 2015 C++ Build Tools no longer installs (deprecated due to sha-1 sig)
- Offline installer requires login, instead base on top of docker image with it already installed: https://hub.docker.com/repository/docker/ericoporto/min-ags-dev-env
@ivan-mogilko
Copy link
Contributor

This fails when building the Editor, from the logs it looks like rc.exe is missing (microsoft resource compiler)?

@ericoporto
Copy link
Member Author

ericoporto commented May 23, 2021

Yeahp, I will look into fixing, it's probably related to have multiple VS installations, so I need to be explicit with the editor.

Edit:
Found out that in the image from our current Dockerfile we use the rc.exe from Windows 8.1 SDK (log below is in my repo, but the image is the same as AGS repo since Cirrus doesn't care who cached the Dockefile)

https://cirrus-ci.com/task/6146950694174720?logs=build#L13

I have it in the docker image built by this PR too, so I just need to find the proper way to have it on the path for MSBuild.

@ericoporto ericoporto changed the title Ci cmake ags36 fixes Ci cmake ags36 fixes [DO NOT MERGE] May 23, 2021
@ericoporto ericoporto marked this pull request as draft May 24, 2021 02:30
@ericoporto
Copy link
Member Author

@morganwillcock when you have time, can you take a look into this PR? I will left it as draft until then.

@ericoporto ericoporto mentioned this pull request May 25, 2021
37 tasks
@morganwillcock
Copy link
Member

I think this is the only option left to use the existing CI configuration, so if it works it is fine with me. I think the only real change would be that previously the caching of the built container would only have been dependent on the Dockerfile contents, but now if we wanted to update something in your container the Dockerfile needs to also change in order to trigger the rebuild - so as long as the container version number changes it should be fine.

Worst case we switch to something else, I've started looking at build system changes so hopefully things get easier to build and not harder.

@ericoporto ericoporto marked this pull request as ready for review May 25, 2021 19:14
@ericoporto
Copy link
Member Author

ericoporto commented May 25, 2021

Thanks Morgan! Yeah, in the meantime I am also looking to understand better how the Vs 2019 build tools work.

@ivan-mogilko can you confirm the binaries generated in the CI (in this PR) are fine on Window 7? It should be (I mean, if it worked before), but I don't have a Win7 machine anymore to test.


forgot to update SDL_sound version on Linux, bug in Docker there now, working on it... (my local docker had that line cached)

Edit: ok, turns out curl in Debian Jessie is too old to be able to download from Xiph since they updated their tls config.

Edit2: Xiph has official mirrors in GitHub, but turns out they don't have libtheora there... (the source provided doesn't match the tarball and fails with many errors when building). Edit3: managed to fix, see xiph/theora#16

@ericoporto ericoporto changed the title Ci cmake ags36 fixes [DO NOT MERGE] Ci cmake ags36 fixes May 25, 2021
- Official Xiph website switched to newer tls config incompatible with curl from Debian Jessie
- add autoconf needed for theora source in debian jessie, needed for Theora GH package
@ericoporto
Copy link
Member Author

ericoporto commented May 28, 2021

Updated SDL_sound on the Linux Dockerfile too, had forgotten about it! Had to do a bit more there since the curl in it is too old (see top post).

Also bsdtar on Windows isn't reliable from piped input when it's a zip file (it's fine for tar.gz), so made it write to disk instead, probably will fix all those "missing AGS.Control.dll" bugs we keep getting on our CI.

Updated the description on top to reflect that.

Let me know if there are any changes on the commits like rebasing or something, but I believe it's all correct now!

These commits look safe for ags4 too.

@ericoporto ericoporto changed the title Ci cmake ags36 fixes CI fixes and updates May 28, 2021
@morganwillcock
Copy link
Member

Also bsdtar on Windows isn't reliable from piped input when it's a zip file (it's fine for tar.gz), so made it write to disk instead, probably will fix all those "missing AGS.Control.dll" bugs we keep getting on our CI.

It is likely a good idea to switch this although you should probably quote the paths just in-case the CI runner ever happens to use a temporary path with a space in it. e.g. "%TEMP%\binaries.zip" instead of %TEMP%\binaries.zip

@ericoporto
Copy link
Member Author

ericoporto commented May 28, 2021

Ah, yes, I can quote. I ended up in a rabbit hole found out it's a limitation when a zip has multiple files, the tar.gz works because of it's two step approach.

@ivan-mogilko
Copy link
Contributor

can you confirm the binaries generated in the CI (in this PR) are fine on Window 7

Yes, it works.

@ivan-mogilko ivan-mogilko merged commit 1df2913 into adventuregamestudio:master May 30, 2021
@ericoporto
Copy link
Member Author

ericoporto commented May 30, 2021

if you want to, for ags4, I have rebased these same commits in the branch here: https://github.com/ericoporto/ags/tree/ci_cmake_ags4_fixes

I don't know how you handle the multiple branches, so I don't know if it helps.

@ivan-mogilko
Copy link
Contributor

I'll just merge the master there, any changes specific to ags4 may be appended separately.

@ericoporto
Copy link
Member Author

Ah, cool, it should automatically merge then, here I had no conflicts!

@ericoporto ericoporto deleted the ci_cmake_ags36_fixes branch June 6, 2021 20:29
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.

SDL2: particular OGG clip does not play
3 participants