search

puzzlepaint / freeage

An open-source reimplementation of the game engine of Age of Empires 2: Definitive Edition.
59 stars 4 forks source link

Enable automatic deploys via CI #12

Closed grst closed 4 years ago

grst commented 4 years ago

This PR configures the CI to

This works in principle (i.e. the files are created), however there are some issues:

MaanooAk commented 4 years ago

Just to note that the linux build fail is fixed in #10.

puzzlepaint commented 4 years ago

Regarding the Windows binary, one could try running windeployqt on the compiled .exe. This is being done for the test binary already and was necessary to be able to run it. I think this copies the necessary .dlls into the binary folder, including the Qt plugins. Perhaps the test binary does not reference Qt5Network, so it is not being copied so far.

grst commented 4 years ago

Hmm, that resolved the issue, but now another library is missing: grafik

puzzlepaint commented 4 years ago

The file name indicates that this is a runtime library from Visual Studio. You could try installing this redistributable package, which should likely fix it: https://support.microsoft.com/en-us/help/2977003/the-latest-supported-visual-c-downloads

I am a bit surprised though that this does not seem to be present on the system already. One way to provide it without having users download this from Microsoft would be to make an installer package that includes it. Perhaps another way could be to link this statically if possible. I don't know what the best way is here.

grst commented 4 years ago

However, the binary you built manually ran without issues on the same system. So there must be a way of including the library...

On Sat, 18 Apr 2020 at 18:59, Thomas Schöps notifications@github.com wrote:

The file name indicates that this is a runtime library from Visual Studio. You could try installing this redistributable package, which should likely fix it:

https://support.microsoft.com/en-us/help/2977003/the-latest-supported-visual-c-downloads

I am a bit surprised though that this does not seem to be present on the system already. One way to provide it without having users download this from Microsoft would be to make an installer package that includes it. Perhaps another way could be to link this statically if possible. I don't know what the best way is here.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/puzzlepaint/freeage/pull/12#issuecomment-615903753, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABVZRV77RZLJCSXOMLHRBW3RNHMALANCNFSM4MLM4UOQ .

grst commented 4 years ago

Nevermind, it was indeed a problem with my runtime library.

I had aborted the installation of Visual Studio previously and that seems to have screwed it up. Reinstalling the redistributable solved the problem and the CI-bundled binary works like a charm now :)

puzzlepaint commented 4 years ago

Okay. It might still be interesting to check whether the version that I compiled manually differed somehow. I don't remember making any manual changes to its settings, but perhaps it was set to link the runtime statically for some reason. I will have a look the next time I boot Windows on this PC.

I think we can additionally prune a few files from the archive to make it smaller. How about something like this in the Release directory:

rm *.lib
rm *.exp
rm Qt5Svg.dll
rm opengl32sw.dll
rm FreeAgeTest.exe

Not sure what opengl32sw.dll is, "sw" seems like it could stand for software rendering, in this case this should definitely never be used ;) Also, not sure why the Qt deployment copies the svg library over. Perhaps this is for supporting this file format hypothetically. But we don't need it for now.

puzzlepaint commented 4 years ago

I checked the manual setup now. The relevant option in Visual Studio is set to "Multi-threaded DLL" (/MD) and the executable depends on "VCRUNTIME140.DLL" (without "_1") according to Dependency Walker. So I guess it was just luck that this runtime was installed already, but the "_1" version was not. I guess that I should simply add a notice for the next release telling users to install the redistributable package if they get an error like that.

grst commented 4 years ago

I also found this claiming that by setting it to /MT, the libraries would be statically linked. But I guess adding a note to the README/release section is fine as well.

grst commented 4 years ago

cd build && ctest still fails after merging the latest master version into this branch. Note that ctest is currently not executed by the CI on the master branch.

MaanooAk commented 4 years ago

@grst I can't reproduce the fail in my machine, try changing the command to ctest -VV for a more verbose output so we may understand what is happening.

puzzlepaint commented 4 years ago 
1: qt.qpa.screen: QXcbConnection: Could not connect to display 
1: Could not connect to any X display.

I would try starting the test with QT_QPA_PLATFORM=offscreen set. (Maybe this could be set as the default for both the test and the server.)

grst commented 4 years ago

:+1:

I was trying to setup Xvfb, but setting the platform to offscreen is probably even better.

On Mon, 20 Apr 2020 at 13:29, Thomas Schöps notifications@github.com wrote:

1: qt.qpa.screen: QXcbConnection: Could not connect to display 1: Could not connect to any X display.

I would try starting the test with QT_QPA_PLATFORM=offscreen set. (Maybe this could be set as the default for both the test and the server.)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/puzzlepaint/freeage/pull/12#issuecomment-616491210, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABVZRV3IX7LWVO267MU7Q63RNQWY5ANCNFSM4MLM4UOQ .

grst commented 4 years ago

QT_QPA_PLATFORM=offscreen seems to have done the trick!

I think this can be merged then (please "Squash & merge", most commits are meaningless and just used for debugging the CI)

puzzlepaint commented 4 years ago

Nice, thanks for working on this! I think we should still attempt the cleanup for the files included in the archive suggested above, i.e., something like

rm *.lib
rm *.exp
rm Qt5Svg.dll
rm opengl32sw.dll
rm FreeAgeTest.exe

but this can be done afterwards.