melonjs / melonJS

a fresh, modern & lightweight HTML5 game engine
https://melonjs.org
MIT License
5.93k stars 643 forks source link

added build helper docker to generate webdoc with correct relative paths #1218

Closed Waltibaba closed 9 months ago

Waltibaba commented 9 months ago

Documentation build helper via a docker container to work around broken webdoc package's absolute-path html anchor generation. See issue https://github.com/melonjs/melonJS/issues/1216. Previous PR #1217 was reverted due to environment namespace pollution with npm docker package, this adds a check to make sure correct docker is being called (using parsed output of docker info command).

Run npm run doc to generate docs with correct relative hyperlinks.

Merge Checklist

Irrelevant, this is an infrastructure change that doesn't affect any source code.

obiot commented 9 months ago

thank you again @Waltibaba, I will merge asap, just got busy yesterday and could not look at it !

Waltibaba commented 9 months ago

I also tested generating with jsdoc using https://github.com/clenemt/docdash as a template instead of using webdoc (since it looks similar and is structured similarly), it looks great locally. I didn't delve into details concerning relative paths (github pages etc.), but that shouldn't be too complex, jsdoc has great documentation. If I have a bit of time I might take a look at that too if you like, or one of the other suggested jsdoc web templates (see https://github.com/jsdoc/jsdoc readme.md).

obiot commented 9 months ago

that would be amazing, and a better more long-term solution !

we previously used the tui-jsdoc template with JSDoc (see here : https://github.com/melonjs/jsdoc-template) if this can help in any way.

I also liked the better-docs one : https://github.com/SoftwareBrothers/better-docs?tab=readme-ov-file

obiot commented 9 months ago

Hi @Waltibaba , sorry but I need your help with this again,

It all worked perfectly the first time I generated the documentation, but now I'm having this error :

> melonjs@17.0.0 doc
> docker/generate-docs.sh

WARNING: daemon is not using the default seccomp profile
[+] Building 0.9s (1/2)                                                        docker:desktop-linux
[+] Building 1.0s (2/2) FINISHED                                               docker:desktop-linux
 => [internal] load build definition from Dockerfile                                           0.0s
 => => transferring dockerfile: 153B                                                           0.0s
 => ERROR [internal] load metadata for docker.io/library/node:lts-buster                       0.9s
------
 > [internal] load metadata for docker.io/library/node:lts-buster:
------
Dockerfile:1
--------------------
   1 | >>> FROM node:lts-buster
   2 |     
   3 |     ENV DEBIAN_FRONTEND=noninteractive
--------------------
ERROR: failed to solve: node:lts-buster: error getting credentials - err: exit status 1, out: ``
melonjs-doc

do you have any idea what it could be ?

also, I did realize docker need to be launched every time (I mean it makes sense, but this to say that definitely a better long term solution is definitely to migrate away from the unsupported webdoc)

Waltibaba commented 9 months ago

error getting credentials refers to your logged-in docker registry. Could it be that you are logged-in to the github one (ghcr) or somewhere else? On docker hub (which is the implicit default non-logged-in container repo), node:lts-buster is available: https://hub.docker.com/layers/library/node/lts-buster/images/sha256-146ac69309e1d88804dc9534d05157c599f67da609eb1e6dd6678189a30dd730?context=explore

It also doesn't require logging in, and if you are logged in somewhere else, I've experienced that causing issues.

obiot commented 9 months ago

I did log in (after signing-up with my GitHub account) actually. I just tried again but it's the same, and as I was saying the very first time it worked as expected.

Waltibaba commented 9 months ago

Sorry, I wasn't very clear; you need to use the default docker registry: hub.docker.com, not the ghcr.io registry. The docker hub is freely available and non-authenticated, but I have experienced problems with the official docker images not found or login errors when I was logged into a private company docker registry.

This setting is set by the docker daemon, which on Mac is set via the Docker Desktop app, though I think login still has to happen via shell command: docker login or docker logout.

So there could be 2 possible solutions:

  1. either try logging out of all docker registries, or
  2. add docker.com/ in the docker FROM line, like:
    FROM docker.com/node:lts-buster
    ...

    The second makes more sense for you, but might not work for someone that has a paid plan for a corporate docker registry, so it's not a foolproof option I suspect.

obiot commented 9 months ago

getting this though now :

WARNING: daemon is not using the default seccomp profile
[+] Building 3.9s (2/2) FINISHED                                                                 docker:desktop-linux
 => [internal] load build definition from Dockerfile                                                             0.0s
 => => transferring dockerfile: 164B                                                                             0.0s
 => ERROR [internal] load metadata for docker.com/node:lts-buster                                                3.9s
------
 > [internal] load metadata for docker.com/node:lts-buster:
------
Dockerfile:1
--------------------
   1 | >>> FROM docker.com/node:lts-buster
   2 |     
   3 |     ENV DEBIAN_FRONTEND=noninteractive
--------------------
ERROR: failed to solve: docker.com/node:lts-buster: encountered unknown type text/html; children may not be fetched
melonjs-doc

im still very confused on why this was working the very first time :(

Waltibaba commented 9 months ago

Ok this seems to be an issue specific to docker desktop on Mac not being set up correctly. Your original error has some troubleshooting on the docker forum. Are you running docker with sudo? In that case, set up your own user to be able to use docker instead (that's how it is supposed to be used), and give the directory the correct permissions for your user. Also found this fix which means resetting docker desktop settings.

obiot commented 8 months ago

man, I hope you won't be offended, but this is getting way over too complicated just to replace a part of a string with another one, so i just wrote a small python script that runs after the doc is generated, and it just does the job fixing all urls. So I'm gonna stick with that for now.

Waltibaba commented 8 months ago

Not at all, I completely understand! Docker is a very Linux-specific tech and I'm having similar problems with it on WSL2 in another project...