Add docker support

[KagurazakaNyaa]

[CLAassistant]

All committers have signed the CLA.

[mdziekon]

Hi @KagurazakaNyaa, thank you for your contribution! This addition will definitely greatly improve development experience for all potential contributors, myself included. Incidentally, this will also close an old issue #16, which is great.

I'll try to review & test this locally during the upcoming weekend, however one thing that already comes up to my mind about this is related to the webserver you've used in the Dockerfile. Seems like you've used Apache (https://github.com/mdziekon/UniEngine/pull/251/files#diff-dd2c0eb6ea5cfc6c4bd4eac30934e2d5746747af48fef6da689e85b752f39557R1). Since I'm not really experienced with Apache, initially I was thinking about using Nginx as the base webserver for the dev environment, mostly because I'm already familiar with it and use it professionally on a day to day basis.

Therefore, my question is - could you refactor your Pull Request to use Nginx instead of Apache? Locally, I've been using richarvey/nginx-php-fpm as a quick solution for a local dev environment, so that might be a good place to start. But... I haven't really done any research beyond that, so there might be something better.

[KagurazakaNyaa]

Ok, I'm using apache because it's a native tag for the _/php image, I'll modify it to a multi-container architecture, using php:7.3-fpm and nginx:stable.

[KagurazakaNyaa]

I created a commit to use php:7.3-fpm and nginx:stable in place of php:7.3-apache, I tested it locally, but it may not be comprehensive. Maybe you can test further to determine if there is a potential bug.

I did not directly share /var/www/html between the two containers, I built the basic content in the composer image, and then mounted the parts that need to be persisted externally, and /var/www/html/tmp is shared using docker volumes. But given that the nginx container and the php container use different users, I'm not sure if there is a permissions issue, but in my tests locally it works fine.

[mdziekon]

My initial review & testing of your PR shows no problems so far, however I didn't have enough time to test any places that could potentially be problematic with permissions (eg. places which save something to the drive, except for the installation script). I'd like to take some more time for testing, but overall I think it should be mergeable, once I'm done with the testing. Since I'm going on vacation in a few days, unfortunatelly this will have to wait until next week, sorry for that :(

In the meantime, if you don't mind, some things could be improved (I'll leave some comment / proposals). However, none of them are show stoppers, so if you don't feel like changing them right now, I can do that myself after we merge your original proposal.

Anyway, great job!

[mdziekon]

@KagurazakaNyaa I was finally able to test your PR more thoroughly, with the most recent changes. So far, the only problems I've found seem to be non-fatal, and are related to write permissions to some of the cache directories:

• generate_sig.php script is unable to access & write anything related to Signatures, leading to broken Signature images (as seen in the Overview page).
• admin/autostatbuilder.php is unable to write to cache/data/last_stats_update.php file. This does not break the Statistics calculation script, but throws a warning on each run, and if I recall correctly, this file is being used in the generate_sig.php file.

I haven't found other scripts that break like that, so right after fixing that (and the sudo comment in README), this PR should be ready for merge :tada:

[KagurazakaNyaa]

Maybe the problem with the cache wasn't a write permission issue, but it wasn't properly shared between the two containers, which I solved by adding a shared volume. When I visit the overview, I can notice in the php log that /generate_sig.php has returned 200.

172.19.0.3 -  13/Sep/2022:09:03:30 +0800 "GET /generate_sig.php" 200
172.19.0.3 -  13/Sep/2022:09:03:28 +0800 "GET /overview.php" 200

[KagurazakaNyaa]

generate_sig.php still seems to have issues, it won't find some files, I need further research to fix this, please don't merge for now, I'll create some more commits to fix it

[KagurazakaNyaa]

The error in generate_sig.php is because the target directory does not exist correctly. In a non-cross-container environment, this is not a problem. But when we need to share a volume across containers, it may have some subdirectories that do not exist, so we need to do extra processing for this situation.

[mdziekon]

What's the current status of your work here? Does the "please don't merge" comment still apply?

---

As far as I can tell, the previous signature generation problem has been fixed, however now it thrown an error about a missing function:

Fatal error: Uncaught Error: Call to undefined function imagettfbbox() in /var/www/html/generate_sig.php:167

Looking at the code and the installed extensions in the Docker files, I have no clue why this is occuring, as all the necessary things are seemingly there.

---

As for the stats calculation problem, it's still there, however the "mkdir in PHP" approach seems to work on that problem as well.

[KagurazakaNyaa]

If nothing else, I think it's ready to be merged. But I also have no clue about the problem with the imagettfbbox function, and if it does not cause any symptoms, I think it can be ignored for now.

[mdziekon]

I agree, signatures are a non-essential feature (and works fine on other environments), so merging it is still beneficial overall. I'll create a ticket for it, to be fixed in the future. As for the stats generation, that can be fixed in a separate PR as well.

[mdziekon]

@KagurazakaNyaa PR has been merged, thank you again for your contribution!

[mdziekon]

Follow-up issues created:

• https://github.com/mdziekon/UniEngine/issues/253
• https://github.com/mdziekon/UniEngine/issues/254