melonjs / melonJS

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

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

Closed Waltibaba closed 7 months ago

Waltibaba commented 7 months ago
Description of change

Documentation build helper via a docker container to work around broken webdoc package's absolute-path html anchor generation. See issue #1216. Run cd docker && ./generate-docs.sh to generate the webdoc docs with correct line-number references in html.

Merge Checklist

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

Waltibaba commented 7 months ago

Let me know if I should change anything (the generated urls seem to be from a Mac?). Either run this locally pre-commit or within a CI/CD for doc deployment on the github pages; travis/github actions will need docker access then however. Feel free to ask if I should adapt to one of those.

obiot commented 7 months ago

that's great ! No I'd rather keep it locally as it will be "easier" to revert the day we actually move on from webdoc (or if the maintainers come back from wherever they are).

One more thing if I can ask, can you also modify the "doc" script in package.json to automatically generate and call this script ? https://github.com/melonjs/melonJS/blob/016e6d65f4b015d07284060a733ef8d8166cd215/package.json#L99

this to include everything when making a release.

Thank you very much for this, really !

Note: and yes it's from a Mac :P

Waltibaba commented 7 months ago

Done, with shell $PWD before/after call preserved. This puts a hard requirement on docker (and a relatively fast internet connection for docker build) for the doc command though. But you can still run doc-prod locally though.

obiot commented 7 months ago

@Waltibaba Hi Sorry, I initially merged it but then I had to revert all commits, as it was not working. the script is generating four files under docker/doc but nothing is actually happening to the doc themselves. Did I missed a step or something ?

also docker need to be added to the devDependencies in the package.json file

Waltibaba commented 7 months ago

Are you running npm run doc from the root of the repo? It still works fine for me. The script should not generate anything in the host ./docker, it builds the container, then runs it with npm run doc-prod, then copies the resulting /docs folder to the host ./docs, so relative path .. .

Sorry about the devDependencies, I mentioned the new dependency on docker before but didn't know that's how non-JS deps are managed in npm.

I updated it to use the existing repository on the host rather than cloning master (that was not fully thought-through...). Also now uses nodeJS docker image as a base instead of plain ubuntu.

Waltibaba commented 7 months ago

Should I create a new PR or do you want to reopen this one? I committed on my fork and merged your latest master, looks like no conflicts.

Waltibaba commented 7 months ago

Ok I'm getting the same issue now: In package.json, scripts section:

...
    "doc": "docker/generate-docs.sh",
...

running npm run doc shows melonjs version and generates some weird css and js files in a /doc folder

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

(node:3361363) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
<bla bla bla>

running docker/generate-docs.sh generates the docs correctly. Once again, npm is doing something weird and inexplicable.

obiot commented 7 months ago

Should I create a new PR or do you want to reopen this one? I committed on my fork and merged your latest master, looks like no conflicts.

yes, I'm afraid of, sorry. Since I merge the pull request (and then reverted the commits), I don't see the option here to re-open it

Waltibaba commented 7 months ago

Ok so we are misunderstanding each other apparently. There is an npm package called docker; https://www.npmjs.com/package/docker . This is some ancient documentation tool, maybe a little bit like webdoc. My implementation uses the linux cgoup based containerization tool docker https://www.docker.com/ .

These are completely different things.

If you run npm install docker, running docker within any npm run script will call that npm package instead of the system tool docker, and generate those 3-4 weird files.

I mentioned

but didn't know that's how non-JS deps are managed in npm

because that's not how they're managed, npm can't manage them. The system dependency on docker just has to be installed separately, maybe mentioned in README.md.

As before, my previous fork as well as the current one both work fine in a clean environment (i.e. fakeroot, fresh ubuntu/debian/etc. install, clean docker container). Global namespace pollution is causing this issue, you likely have the npm package docker installed in your node_modules in here, for your user or globally.

I'm still not sure what your CI is or if you're running it from your workstation, so I can't hard code the docker path (since it would vary based on operating system, etc.). If you want any help at all, please message me.

obiot commented 7 months ago

Ok so we are misunderstanding each other apparently. There is an npm package called docker; https://www.npmjs.com/package/docker . This is some ancient documentation tool, maybe a little bit like webdoc. My implementation uses the linux cgoup based containerization tool docker https://www.docker.com/ .

okay yeah, I completely missed that one :)

I'm still not sure what your CI is or if you're running it from your workstation, so I can't hard code the docker path (since it > would vary based on operating system, etc.). If you want any help at all, please message me.

it's on my workstation, we used to use Travis at some point, but actually never for the documentation.