dev-php/phpunit: version bump, fix bug 639656

[kitsunenokenja]

Add ebuild for current stable release from project upstream.

Closes: https://bugs.gentoo.org/639656
Package-Manager: Portage-2.3.13, Repoman-2.3.3

[gentoo-repo-qa-bot]

Pull Request assignment

Areas affected: ebuilds
Packages affected: dev-php/phpunit

dev-php/phpunit: @gentoo/php

Bugs linked: 639656

[orlitzky]

That "phar" file is a zip file containing not only PHPUnit, but also a copy of every library it uses (and every library that those library use...). To package this properly, you'll have to create separate packages (i.e. ebuilds) for them, and then ultimately have PHPUnit-6.x depend on those packages.

@GuillaumeSeren is working on this, so you'll probably want to talk to him if you're interested in that (much larger) task.

[kitsunenokenja]

@orlitzky, thanks for exposing this fact. It was not directed to my attention when I checked the upstream resources. Their manual only refers to PHP itself and components of it that must be present, neglecting to elaborate that their PHAR is a collection beyond their own sources.

@GuillaumeSeren could you please comment on this PR? It would be counterintuitive for me to repeat everything you've done thus far. The absence of >phpunit-6 in portage is a dependency block for another ebuild I want to introduce, so I'm looking to have this one cleared up.

[GuillaumeSeren]

@kitsunenokenja Hello,
packaging big php component can take a long time but, it can be done I managed to finish composer,
and I would like to finish full packaged phpunit as well, so I could add tests feature to composer.

If want to help working on that subject it could be great,
the thing is we cant merge half way, so we need to package all deps of phpunit and so on (+ tests when we can).

To track my progress I try to keep my overlay up to date (https://github.com/GuillaumeSeren/gentoo-overlay), but I have been working on others subjects and this is not moving fast so again if want to contribute on this I can review you work and help if you have questions.

To have some more precise idea about the work itself I suggest you read the composer issue in the BTS 439206 (https://bugs.gentoo.org/439206) and more precisely those commits

d2730461386 dev-php/composer: Add 1.2.2 version [Guillaume Seren]                                                                                                                                                                              
bc36d09d052 dev-php/symfony-console: Add 2.7.9 version [Guillaume Seren]                                                                                                                                                                       
2283ea23104 dev-php/symfony-event-dispatcher: Add 2.1.0 version [Guillaume Seren]                                                                                                                                                              
9963181de94 dev-php/symfony-dependency-injection: Add 2.1.0 version [Guillaume Seren]                                                                                                                                                          
e751acece31 dev-php/symfony-yaml: Add 2.1.0 version [Guillaume Seren]                                                                                                                                                                          
d10d7e2f7e3 dev-php/symfony-config: Add 2.1.0 version [Guillaume Seren]                                                                                                                                                                        
7597666e5c4 dev-php/ca-bundle: Add 1.0.0 version [Guillaume Seren]                                                                                                                                                                             
0bccd72e3e2 dev-php/semver: Add 1.0.0 version [Guillaume Seren]                                                                                                                                                                                
a78020686e9 dev-php/spdx-licenses: Add 1.0.0 version [Guillaume Seren]                                                                                                                                                                         
033f5d398db dev-php/jsonlint: Add 1.4.0 version [Guillaume Seren]                                                                                                                                                                              
79a2aeb43a0 dev-php/json-schema: Add 2.0.0 version [Guillaume Seren]                                                                                                                                                                           
d3bd35121b5 dev-php/cli-prompt: Add 1.0.0 version [Guillaume Seren]                                                                                                                                                                            
ea07d004365 dev-php/phar-utils: Add 1.0.0 version [Guillaume Seren]                                                                                                                                                                            
57de338f5b6 dev-php/symfony-finder: Add 2.7.20 version [Guillaume Seren]                                                                                                                                                                       
bdc66737b4f dev-php/symfony-filesystem: Add 2.7.20 version [Guillaume Seren]                                                                                                                                                                   
a7733e14f60 dev-php/symfony-process: Add 2.8.12 version [Guillaume Seren]                                                                                                                                                                      
6c369a57c2e dev-php/psr-log: Add 1.0.2 version [Guillaume Seren]                                                                                                                                                                               
38e69d1f39f dev-php/fedora-autoloader: Add 0.2.1 version [Guillaume Seren]

[kitsunenokenja]

@GuillaumeSeren, excellent. Portage already has a handful of the dependencies that phpunit calls for, and I also see some of them in your overlay. By incorporating what you have thus far I could round this up faster. I'm building out a list of all the deps before determining which ebuilds we're going to need. Some will be version bumps and a small number will be new ebuilds, I think, based on first glance.

Also, unless there is valid objection, I'd like to have a USE flag control the option to just install the PHAR which would bypass all the dependencies. That way users can install normally by default or opt into taking upstream's packaged PHAR.

[orlitzky]

Also, unless there is valid objection, I'd like to have a USE flag control the option to just install the PHAR which would bypass all the dependencies.

This is not quite against policy, but it is strongly discouraged; see e.g. https://bugs.gentoo.org/show_bug.cgi?id=bundled-libs

The main reason is that fixes to the dependencies in the tree would not affect the bundled version of PHPUnit. That's especially bad for security fixes, where we put out announcements like "upgrade to version 100 and you'll be safe!" End users might upgrade the library to the safe v100, not realizing that PHPUnit has its own vulnerable copy of v99 (and there's no way for us to track that).

Another reason is "symbol collisions," where you wind up with a name clash. PHPUnit can be used as a library, so imagine I've written a program that uses PHPUnit to read some test cases. If my program also uses one of the libraries that is bundled into PHPUnit, then we might get a situation where PHPUnit tries to load one version of a class, but another version is already in scope because my program used it. Bug 471682) is an example of that. The phar file might not have this problem if it overloads the include order, I don't know off the top of my head.

That said, there are exceptions to the rule. The PHP runtime itself has things like media-libs/gd bundled, but the bundled copy is heavily modified, and it would be incredibly hard to fix at this point. And the PHP project puts out separate security bulletins for their bundled copy, avoiding (well, with twice as much work) that particular problem.

In this case the bundled libraries are 100% vanilla copies, though, so I'd rather not go down that road again.

[kitsunenokenja]

Six more dependencies to address based on my last count, plus the revision of the phpunit-6.5.5.ebuild itself to go.

[kitsunenokenja]

it is strongly discouraged

Very good points. Safe to say I won't include the PHAR as an option in the ebuild. Thank you for the details!

[GuillaumeSeren]

@kitsunenokenja
Having a common list could be great and we could use my overlay if you want to host the work in progress and share it easily (or setup one for you).
I am happy to share the amount of work that way we can be faster and make more packages.

About the USE flags to get a phar version, the subject as been discussed before, and to my understanding we try to avoid a phar version, at least in term of control the deps need to build it,
but @orlitzky can probably say more about that than me (and you also have that covered in the composer isssue linked before).

[kitsunenokenja]

@GuillaumeSeren, can you address the phpunit deps phar-io/manifest and phar-io/version? They are probably very simple. Also, theseer/tokenizer as you already did 2 from theseer before, so this one will also be quick for you I'm sure.

EDIT: I have all the other sebastian-* deps ready. All I need is php-code-coverage which depends on theseer/tokenizer; as soon as you have that posted I can tie this all up with the last round of ebuilds.

[GuillaumeSeren]

@kitsunenokenja
Yes I think I could, feel free to PR your stuff in my overlay that way we can track the progress.

[kitsunenokenja]

@GuillaumeSeren you have some packages whose entry names do not match Portage's existing classifications for the same things. It was actually quicker to do the version bumps on everything that Portage already knew about based on what's already in the tree. There will be extra overhead to make a PR of this for your overlay as a result which is counterintuitive, I'm sure you'll understand.

If you want to work through it in the other direction and update your overlay accordingly in order to account for the commits in my branch instead if you feel it would help?

[GuillaumeSeren]

@kitsunenokenja
I am working full time right now and I have not enough time to help on packaging so maybe I will just help in the review, as you seems to move forward pretty quick.

[orlitzky]

Another reason is "symbol collisions," where you wind up with a name clash. PHPUnit can be used as a library... The phar file might not have this problem if it overloads the include order, I don't know off the top of my head.

To answer myself, this question doesn't even make sense. Although PHPUnit can be used as a library, the phar file can't. So it's not at risk of symbol collisions, but only because there's a bigger problem: other applications have no way to include 'PHPUnit/whatever.php'; from the phar. We have at least one package doing that, dev-php/phing.

[kitsunenokenja]

Yes if you were to only include the libraries ordinarily but not the PHAR, you shouldn't have a conflict. But you did mention a security concern and it would be very obscure to track PHAR-based ebuilds in that regard to ensure everyone inherits updated versions. That alone is reason enough to not offer the option.

I'll carry on and wrap up the whole PR myself and ping you, orlitzky, when I'm ready. I realised some of the ebuilds reference the PHP deps without considering PHP 7.2 which is valid, so I will also be going back to update those as well.

[orlitzky]

A lot of these packages have the HOMEPAGE set to http://phpunit.de, but that now redirects to https://phpunit.de/ (with an 's'). Might as well update them to save the environment =)

[kitsunenokenja]

Good catch, thanks. Took care of the URLs. Had a minor mistake with dependency resolution that portage pointed out, fixed it, now the dep tree builds out right without issue. I'll be testing soon as I expect I may not have set all the autoload.php files right just yet.

Then I presume I'll have to do commit squashing to simplify the history. Almost there. This task hasn't been awfully tremendous. There are a lot of little ebuilds involved sure, but it's straightforward for them all.

[kitsunenokenja]

@orlitzky, please have a look at the current state of the PR. The last set of changes are set, and I've installed and successfully run phpunit. Looking to make any outstanding corrections first before squashing everything.

[orlitzky]

I don't see anything wrong, can you please squash everything?

Once the commits look nice, I'll merge then one at a time and test them fully. Then if I get halfway through the list of commits and find a problem, we don't have to start over -- you can just rebase onto master.

[kitsunenokenja]

@orlitzky, rebase complete.

EDIT: It just occurred to me after preparing this that the change to the files/autoload.php file for phpunit-6.5.5 will break the previous 5.7 ebuild since some old entries were taken out that became obsolete in 6.5.5. Unless the decision is to drop the old 5.7 ebuild then this will have to be fixed by moving to a separate file to resolve the conflict. In that case I can update the ebuild for 6.5.5 and refer to the file as files/autoload-6.5.5.php, but merge it into the file system as just autoload.php.

[kitsunenokenja]

@orlitzky, seems those underscores don't work in the autoloader script and I'll just stick to the proper backslash delimiters. I had seen it somewhere else and figured those strings would be okay that way. Well, now we know.

Also, your test code has a bug. You can't call echo with the result of the parse method because it returns an object. Please use this revised version; it is fixed and passed the test:

<?php
require_once '/usr/share/php/PharIo/Version/autoload.php';

use PharIo\Version\Version;
use PharIo\Version\VersionConstraintParser;

$parser = new VersionConstraintParser();
$caret_constraint = $parser->parse( '^7.0' );

var_dump($caret_constraint->complies( new Version( '7.0.17' ) )); // true
var_dump($caret_constraint->complies( new Version( '7.1.0' ) )); // true
var_dump($caret_constraint->complies( new Version( '6.4.34' ) )); // false

$tilde_constraint = $parser->parse( '~1.1.0' );

var_dump($tilde_constraint->complies( new Version( '1.1.4' ) )); // true
var_dump($tilde_constraint->complies( new Version( '1.2.0' ) )); // false

If we see another autoloader issue I'm going to update all of them that are outstanding in the PR the same way I fixed these two phar-io packages.

[orlitzky]

In hindsight, the PharIo/Version autoloader can be a lot simpler. The "classMap" autoloader is only needed for the packages that have a nonstandard layout. The PharIo/Version scheme maps classes to files according to PSR-4, so the following should suffice:

<?php
/* Autoloader for dev-php/phar-io-version */

require_once 'Fedora/Autoloader/autoload.php';

\Fedora\Autoloader\Autoload::addPsr4('PharIo\\Version\\', __DIR__);

Some of the others are probably similar. Sorry for not noticing that earlier!

[kitsunenokenja]

Oh great, it does have a sensible PSR-4 aware routine. Made the switch. We can't use this for the other PharIo because it doesn't follow the pattern between filesystem and namespacing.

[orlitzky]

Ok, I merged phar-io-version, but now the autoloader for phar-io-manifest needs work =)

It's at least missing the dependency on the autoloader for phar-io-version that will look something like this (untested):

\Fedora\Autoloader\Dependencies::required(array(
    '/usr/share/php/PharIo/Version/autoload.php',
));

Afterwards please check that the example in the README at https://github.com/phar-io/manifest can be run.

[kitsunenokenja]

Glad you asked me to test as it saved us both a little time. The test exposed some issues in the autoloader after having added the dependency statement. The sample code from your link will require an input file (as seen in the source) to run, and the phpunit tests for the project use this:

https://raw.githubusercontent.com/phar-io/manifest/master/tests/_fixture/phpunit-5.6.5.xml

Made the changes to the autoloader for this package after passing the test.

[orlitzky]

Cool, phar-io-manifest is merged. Some of the remaining commits need to be reordered; the new PHP_CodeCoverage for example depends on the new sebastian-environment-3.0.0, which I think is next on my list. Which brings me to... the fact that we need a new autoload.php for the new sebastian-environment. There's a new file in OperatingSystem.php that isn't addressed in the current autoload.php. Since the old sebastian-environment-2.0.0 is marked stable on a few arches, we have to make a new one and not modify the old one.

Some good news, though: I think we can use the addPsr4() method in the new sebastian-environment autoloader, so that this problem won't happen again.

[kitsunenokenja]

phar-io-manifest is removed from PR, and PHP_CodeCoverage is pushed down to come after the group of sebastian-* packages. I've put in the addPsr4 call in the new autoloader in its own file, autoloader-3.1.0.php, the same way I addressed the same issue with phpunit's ebuild.

I wrote this as a quick way to verify the autoloader is resolving properly.

<?php
require_once "/usr/share/php/SebastianBergmann/Environment/autoload.php";
use SebastianBergmann\Environment\OperatingSystem;

echo (new OperatingSystem())->getFamily() . "\n";

[orlitzky]

I've put in the addPsr4 call in the new autoloader in its own file, autoloader-3.1.0.php, the same way I addressed the same issue with phpunit's ebuild.

Thanks, it looks like we'll need to do the same for the PHP_TokenStream autoloader because the existing one is used by the stable PHP_TokenStream-1.4.11.

[kitsunenokenja]

Both 1.4 and 2.0 branches for PHP_TokenStream appear to have classes defined in the same way in the same files. It did not appear that a different file should be made or is needed for the 2.0 ebuild. Did you notice any differences?

[orlitzky]

There may not be any differences (in other words, the test class additions simply fix a bug in the old autoloader), but since the 1.4 version is stable we're not supposed to touch the ebuild or any of its files. "Stable" in Gentoo means "it doesn't change," not "it works right" =)

[kitsunenokenja]

I've been staring at the current version of that particular autoloader so long now, that I've forgotten it isn't the original version of that file. Sorry about that. I'll clean up that part of the PR then and restore the original autoload.php that's associated with the stable ebuild already in place.

[gentoo-repo-qa-bot]

Pull request CI report

Report generated at: 2018-02-15 22:59 UTC
Newest commit scanned: 95ebac4
Status: ✅ good

No issues found

[kitsunenokenja]

@orlitzky, could you provide a status update please?

[orlitzky]

@kitsunenokenja I've been working double shifts at my dayjob and have lost the will to live, a prerequisite for Gentoo work. It will be at least another week before this project is done =(

[kitsunenokenja]

@orlitzky, all right, take your time then. Just checking that this didn't fall through the cracks since this has been a larger undertaking than most PRs, didn't want it to become forgotten! Thanks.

[kitsunenokenja]

@orlitzky, can we reach the finish line with this one? Perhaps I can try finding another able reviewer in #gentoo-php if not.

[orlitzky]

My work situation hasn't changed... you'd better ask around, sorry to drop the ball.