Bug 1413434

Quick notes:

- you've been hit by the ".gitattributes" issue. Upstream remove test suite from the tarball, so you have to use a git snapshot (see various packages, such as the php-zendframework-zend-* ones for example). Running test suite is a must when possible (and allow Koschei monitoring)

- better to add an autoloader (using fedora/autoloader) as composer.json state this library is PSR64 compliant. Will help you:

   - to run the test suite
   - to use this lib. from other package

Hello Remi!

The upstream project already has an autoloader.php for the installed package, and they had a tests/bootstrap.php that was close to what we needed but not quite the same. I ended up using two sed statements to modify the tests/bootstrap.php and bin/export.php so that they require_once with the installed path. What do you think about that approach?

Also, I realized that it seems that they may expect that bin/export.{php,sh} to get distributed with the package and I have not done that so far because the name "export.php" sounds too general to go into /usr/bin. Do you think we should distribute that executable? If so, should I name it something more specific (gettext-languages-export.sh, cldr-to-gettext-plural-rules-export.sh?) The tests rely on this executable running first to generate some data file (see tests/bootstrap.php), which is why I used that sed statement, but if we want to distribute it I'll need to sed a copy of it instead (and sed the one we install separately).

Here's what I have so far, without distributing that executable:

Spec URL: https://bowlofeggs.fedorapeople.org/php-gettext-languages.spec
SRPM URL: https://bowlofeggs.fedorapeople.org/php-gettext-languages-2.1.3-2.fc26.src.rpm

> The upstream project already has an autoloader.php

Indeed, I miss it.

> Also, I realized that it seems that they may expect that bin/export.{php,sh} to get distributed with the package 

Indeed (reading the README)

Notice: this means that the comopser.json is incomplete, should have this listed in the "bin" section (ex, see https://github.com/sebastianbergmann/phpunit/blob/5.7/composer.json#L61).

=> could be submitted upstream

The .sh is uneeded, (the .php with a propoer shebang is enough)

=> using "#!/usr/bin/env php" can be submitted upstream.

(Fedora Guidelines, recommends #!/usr/bin/php, but using env is also ok, if you require php-cli, at least this allow the command to work with SCL... even if this is more correct outside Fedora).

> Do you think we should distribute that executable? 

yes

> If so, should I name it something more specific (gettext-languages-export.sh, cldr-to-gettext-plural-rules-export.sh?) 

Yes. "%{name}-export" seems ok (no suffix)

> but if we want to distribute it I'll need to sed a copy of it instead (and sed the one we install separately).

Indeed. 
Patch it in %prep for installation
Patch it again in %check.

From php-nikic-php-parser

   sed -e 's:%{php_home}:%{buildroot}%{php_home}:' \
      bin/php-parse > bin/php-parse-test

Hello Remi! It was wonderful to meet you at DevConf.cz!

(In reply to Remi Collet from comment #3)
> Notice: this means that the comopser.json is incomplete, should have this
> listed in the "bin" section (ex, see
> https://github.com/sebastianbergmann/phpunit/blob/5.7/composer.json#L61).
> 
> => could be submitted upstream

https://github.com/mlocati/cldr-to-gettext-plural-rules/pull/12

> => using "#!/usr/bin/env php" can be submitted upstream.

https://github.com/mlocati/cldr-to-gettext-plural-rules/pull/13

Upstream had a question about whether the shebang will be printed in the output - if you know the answer it could be helpful to chime in.

> > Do you think we should distribute that executable? 
> 
> yes

Done!


The new code is here:

Spec URL: https://bowlofeggs.fedorapeople.org/php-gettext-languages.spec
SRPM URL: https://bowlofeggs.fedorapeople.org/php-gettext-languages-2.1.3-3.fc26.src.rpm

Oh, one more thing: fedora-review is upset that php-gettext-gettext and this package both own /usr/share/php/Gettext. I don't know a way around that without creating a php-gettext-common package that just owns that directory, or by making php-gettext-languages own it (which just seems weird, but maybe that's OK?) This seems like a problem that might be common with PHP packages - is there a common solution, or is it something that is just ignored?

As php-gettext-gettext requires php-gettext-languages, on the latest have to own this directory.

Created attachment 1247398 [details]
phpci.log

     Note: phpCompatInfo version 5.0.4 DB version 1.17.0 built Jan 24 2017
     09:56:36 CET static analyze results in

Created attachment 1247399 [details]
review.txt

Generated by fedora-review 0.6.1 (f03e4e7) last change: 2016-05-02
Command line :/usr/bin/fedora-review -b 1413434
Buildroot used: fedora-rawhide-x86_64
Active plugins: Generic, PHP, Shell-api

1 minor easy fix:

[!]: Rpmlint is run on all installed packages.
        php-gettext-languages.src:44: W: rpm-buildroot-usage %prep sed -i "s:require_once.*:require_once '%{buildroot}/%{_datadir}/php/Gettext/Languages/autoloa
der.php';:" tests/bootstrap.php
        easy fix => move this in %check


No blocker

[x]: Package complies to the Packaging Guidelines

=== APPROVED ===

Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/php-gettext-languages

php-gettext-languages-2.1.3-3.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2017-aca77236c7

php-gettext-languages-2.1.3-3.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2017-46016c3a74

php-gettext-languages-2.1.3-4.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-aca77236c7

php-gettext-languages-2.1.3-4.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-46016c3a74

php-gettext-languages-2.1.3-4.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report.

php-gettext-languages-2.1.3-4.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report.