phpbb / epv

Extension Pre-Validator
GNU General Public License v2.0
9 stars 17 forks source link

Wrong detection of packaging structure. #33

Closed lavigor closed 9 years ago

lavigor commented 9 years ago

Hello, guys!

Today I've found that the EPV started to push errors Packaging structure doesn't meet the extension DB policies. even when all is OK.

I've explored the latest updates of the EPV and found out that this edit now causes wrong errors.

That's because $this->opendir may be not the same as the actual directory. On my local tests opendir is a relative path, but $dir and $sp are absolute paths. So str_replace() cannot delete the relative path from the absolute path properly.

The new code also works wrongly on Travis and on phpbb.com checker even for official extensions.

I suggest reverting back the edits linked above to the previous code:

                    $sp    = str_replace('\\', '/', $dir);
                    $split = explode('/', $sp);
                    $ns    = '';
                    if (sizeof($split) - 3 >= 0)
                    {
                        $ns .= $split[sizeof($split) - 3] . '/' . $split[sizeof($split) - 2];
                    }

                    if ($this->namespace != $ns)
                    {
                        $this->output->addMessage(Output::ERROR, 'Packaging structure doesn\'t meet the extension DB policies.');
                    }
paul999 commented 9 years ago

No, the old code is wrong as well, as it doesn't correctly detect extra files. The problem is that opendir should be made absolute in the testRunner, this will fix another issue we are encountering as well.

I will look into fixing this tomorrow.

paul999 commented 9 years ago

Just as a note, the checker on phpBB.com might fail (If you use the one on phpbb.com/extensions/epv) if you don't fave the full directory structure in your github repository. The one in titania however should work with valid packages (Having vendor/name).

lavigor commented 9 years ago

...if you don't fave the full directory structure in your github repository.

Then maybe the checker needs an update? It checks only the GitHub repositories and they often don't have the full package structure.

paul999 commented 9 years ago

The checker exists on phpBB.com for informative usage only. EPV is, primarly, meant to pre validate on submission in titania. Besides that, it can be used differently, however it might give wrong results. The ZIP file submitted to the DB should meet the guidelines, and thats where EPV checks for.

phpbbireland commented 9 years ago

I'm getting the same error but no indication of what's actually wrong...

Packaging structure doesn't meet the extension DB policies
and how do I find out? Also, where do I ask about unserialize and htmlspecialchars causing fails (errors) but passed on 3.0? It would be nice to have a pass for such a big extension even if I still have lots of work to reducing globals...(dependency injection look out)...
paul999 commented 9 years ago

Like said, this error is specific for titania, and if you submit it should not give the error. If you package your extension correctly (As per the extension guidelines), it is fine. You can ignore the error if you are running EPV locally or on travis.

Also, having a error is does not mean it is a failure. Several errors are to warn the extension team for something what might be incorrect.

Please ask any further questions in extensions writers.

Dark1z commented 5 years ago

Work around for this in Travis-CI or any CI would be for extension vendor_name/ext_name : from : .../ext/vendor_name/ext_name/<ext_files> to : .../ext/vendor_name/ext_name/vendor_name/ext_name/<ext_files>

Extra Debugging is due to : In File : X:\forum\vendor\phpbb\epv\src\Tests\Tests\epv_test_validate_directory_structure.php

case 'composer.json':
    $files['composer'] = true;

    if (basename($dir) != strtolower(basename($dir)))
    {
        $this->output->addMessage(Output::WARNING, 'The name of composer.json should be completely lowercase.');
    }
    $this->output->addMessage(Output::DEBUG, '1. `$dir` : '.$dir);

    $sp    = str_replace('\\', '/', $dir);
    $this->output->addMessage(Output::DEBUG, '2. `$sp` : '.$sp);
    $sp    = str_replace(str_replace('\\', '/', $this->opendir), '', $sp);
    $this->output->addMessage(Output::DEBUG, '3. `$opendir` : '.$this->opendir);
    $this->output->addMessage(Output::DEBUG, '3. `$sp` : '.$sp);
    $sp    = str_replace('/composer.json', '', $sp);
    $this->output->addMessage(Output::DEBUG, '4. `$sp` : '.$sp);

    if (!empty($sp) && $sp[0] == '/')
    {
        // for some reason, there is a extra / on at least OS X
        $sp = substr($sp, 1, strlen($sp));
    }
    $this->output->addMessage(Output::DEBUG, '5. `$sp` : '.$sp);

    if ($this->namespace != $sp)
    {
        $this->output->addMessage(Output::ERROR,
            sprintf("Packaging structure doesn't meet the extension DB policies.\nExpected: %s\nGot: %s",
            $this->namespace, $sp));
    }
    break;

Before with Err0r :

X:\forum>php vendor/phpbb/epv/src/EPV.php run --dir="ext/dark1/memberavatarstatus"
Running Extension Pre Validator on directory ext/dark1/memberavatarstatus.
Running tests.

 Validation: FAILED
 Fatal: 0, Error: 1, Warning: 0, Notice: 0

Test results for extension:
1. `$dir` : X:\forum\ext\dark1\memberavatarstatus\composer.json
2. `$sp` : X:/forum/ext/dark1/memberavatarstatus/composer.json
3. `$opendir` : X:\forum\ext\dark1\memberavatarstatus
3. `$sp` : /composer.json
4. `$sp` :
5. `$sp` :
Error: Packaging structure doesn't meet the extension DB policies.
Expected: dark1/memberavatarstatus
Got:

After the fix :

X:\forum>php vendor/phpbb/epv/src/EPV.php run --dir="ext/dark1/memberavatarstatus"
Running Extension Pre Validator on directory ext/dark1/memberavatarstatus.
Running tests.

PASSED:  Fatal: 0, Error: 0, Warning: 0, Notice: 0
Test results for extension:
1. `$dir` : X:\forum\ext\dark1\memberavatarstatus\dark1\memberavatarstatus\composer.json
2. `$sp` : X:/forum/ext/dark1/memberavatarstatus/dark1/memberavatarstatus/composer.json
3. `$opendir` : X:\forum\ext\dark1\memberavatarstatus
3. `$sp` : /dark1/memberavatarstatus/composer.json
4. `$sp` : /dark1/memberavatarstatus
5. `$sp` : dark1/memberavatarstatus

Hope it helps someone. Best Regards 👍