phpmyadmin / scripts

Various scripts related to project
16 stars 19 forks source link

Developer documentation fails to build #11

Closed ibennetch closed 6 years ago

ibennetch commented 6 years ago

Our developer docs fail to build:

# /home/builder/scripts/develdocs/vendor/bin/apigen generate --debug  --todo --source libraries/ --destination  /home/builder/scripts/develdocs/output/phpmyadmin

Fatal error: Cannot use 'Object' as class name as it is reserved in /home/builder/scripts/develdocs/vendor/latte/latte/src/Latte/Object.php on line 14
ErrorException: Cannot use 'Object' as class name as it is reserved in /home/builder/scripts/develdocs/vendor/latte/latte/src/Latte/Object.php:14
Stack trace:
#0 [internal function]: Tracy\Debugger::shutdownHandler()
#1 {main}
Unable to log error: Logging directory is not specified.

Looks like a problem with ApiGen: https://github.com/ApiGen/ApiGen/issues/1039

We're on version 4, which is no longer supported. The poster's solution there was to upgrade to the current version 5, (which is still technically in pre-release form).

However, if I try with current dev-master, it fails with a memory error:

vendor/bin/apigen generate  /var/www/pma-dev/fork/libraries/ --destination output
Parsing reflections (this may take a while)...
mmap() failed: [12] Cannot allocate memory
Segmentation fault

I've allocated 8000M and still get errors. Another user had trouble but seems to have resolved it with a memory limit of 512M.

nijel commented 6 years ago

APIGen 5.0 doesn't scale for us (and many other projects), see https://github.com/ApiGen/ApiGen/issues/1015. There is PR to partially address this https://github.com/ApiGen/ApiGen/pull/1038, however the project is looking for new maintainer https://github.com/ApiGen/ApiGen/issues/1020, so it's unsure whether the issue will ever be addressed.

Maybe it's time to look for other docs generator, see https://github.com/cakephp/cakephp-api-docs/issues/99 for similar discussion.

ibennetch commented 6 years ago

I've done a brief test of several of the options and here are my findings along with notes from the referenced tickets:

I can upload the sample output from SAMI if people would like to take a look.

ibennetch commented 6 years ago

I've temporarily disabled the script in the crontab since this isn't working anyway.

williamdes commented 6 years ago

Sami seems to be a nice solution : https://github.com/FriendsOfPHP/Sami Twig uses Sami : https://twig.symfony.com/api/2.x/index.html

I have successfully built the docs of phpMyAdmin with Sami, it looks good (IMO).

@ibennetch Any update to bringing back develdocs ?

nijel commented 6 years ago

@williamdes Can you adjust https://github.com/phpmyadmin/scripts/blob/master/develdocs/build.sh to build docs using sami? Once it works, it will get pushed to develdocs.phpmyadmin.net.

ibennetch commented 6 years ago

The update is basically that when I tested Sami it was the best of the three I tried, but no one else commented about whether the two shortcomings I mentioned were a problem or something that can be worked around, so I didn't take any further action.

As Michal said, you're welcome to submit the pull request as anything is better than what we have now.

williamdes commented 6 years ago

Pull request submitted: #13

nijel commented 6 years ago

I've just created followup issues for all projects to replace using apigen in the CI with Sami.

williamdes commented 6 years ago

@nijel I will fix those issues, do you see any problem with the use of wget or curl to get https://raw.githubusercontent.com/phpmyadmin/scripts/master/develdocs/sami.php and then use it like this ?

./sami.php --root ./ --build-dir ./build/apidocs/ \
    --cache-dir ./tmp --output-config ./sami-config.php \
    --title-of-composer
    # Render
    ./vendor/bin/sami.php update ./sami-config.php

Sami does not have a lint mode (AFAIK).

nijel commented 6 years ago

Won't be better to put that into composer module and install by composer (not sure if that is doable).

The problems I see:

williamdes commented 6 years ago

Yes, maybe a separate repository/module is a better option.

So in that case we would need to do ci-install-apidocs

#!/bin/sh
set -e
composer require "sami/sami:^4.0"
composer require "phpmyadmin/genapidocs:^1.0"

./vendor/bin/pma-genapidocs.php --root ./ --build-dir ./build/apidocs/ \
    --cache-dir ./tmp --output-config ./tmp/sami-config.php \
    --title-of-composer

ci-apidocs

#!/bin/sh
set -e
./vendor/bin/sami.php update ./tmp/sami-config.php
nijel commented 6 years ago

There requirements probably should be already in require-dev...

williamdes commented 6 years ago

Seems like not: https://github.com/phpmyadmin/phpmyadmin/blob/master/composer.json

I think this is intentional.

nijel commented 6 years ago

It is not for phpmyadmin, but it is there for libraries, I'm not sure if this inconsistency really has good reason.

williamdes commented 6 years ago

I know :) All other repositories have it in require-dev, you are right.

You introduced apidocs in Travis with : https://github.com/phpmyadmin/phpmyadmin/commit/1cc0bc6fc918ec5a6a9794cadd097ea7014dae7b

In composer.lock you can see apigen/apigen : https://github.com/phpmyadmin/phpmyadmin/commit/6ba68edd4c94a4d9ca7ff3a4679862a01aae3740

Why apigen/apigen is not in the composer.json of phpmyadmin ?

nijel commented 6 years ago

There is probably no good reason to not have it there.

williamdes commented 6 years ago

@nijel Sami has a lint mode

ERROR: "3" @param tags are expected but only "2" found on "PhpMyAdmin\SqlParser\Components\CaseExpression::parse" in ./src/Components/CaseExpression.php:78

The build fails if an error is found. ref: https://travis-ci.org/phpmyadmin/sql-parser/jobs/388500327#L542

nijel commented 6 years ago

Great, we should have that enabled to catch errors from docs in the CI.

williamdes commented 6 years ago

It is part of Sami, build fails (only exit code) on error. No changes to make :)

nijel commented 6 years ago

Even better ;-)