Bug 701028

Summary: Review Request: php-Smarty-gettext - Gettext support for Smarty
Product: [Fedora] Fedora Reporter: Olivier BONHOMME <obonhomme>
Component: Package ReviewAssignee: Matthias Runge <mrunge>
Status: CLOSED INSUFFICIENT_DATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: unspecified    
Version: rawhideCC: fedora, fedora-package-review, mrunge, notting, obonhomme, pahan
Target Milestone: ---Flags: mrunge: fedora‑review?
Target Release: ---   
Hardware: All   
OS: Linux   
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2013-09-24 04:38:10 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On:    
Bug Blocks: 704672    

Description Olivier BONHOMME 2011-04-30 09:01:18 EDT
Spec URL:  http://ares.ptitoliv.net/~ptitoliv/fedora/smarty-gettext/smarty-gettext.spec
SRPM URL: http://ares.ptitoliv.net/~ptitoliv/fedora/smarty-gettext/smarty-gettext-1.0b1-1.fc14.src.rpm
Description: Smarty gettext plug-in provides an internationalization support
for the PHP template engine Smarty.

Hello, I submit this Review Request to the Fedora Package team in order to integrate this library in the next Fedora Release. It is my first package, so I request a sponsor for my first review process.

Thanks a lot.
Comment 1 Remi Collet 2011-05-14 04:25:31 EDT
Some notes (not a review)

- version is not correct for a pre-release version (1.0-0.1.b1)

- use a simple sed instead of dos2unix (avoid dependency)

- don't install doc, list them in %file is enough
%doc README COPYING Changelog

- what is the goal of the 01_fix_binary_license.patch ?
only to change "space"  (diff -b is empty) ?
I don't think it is really usefull... 

- name is not correct
and http://fedoraproject.org/wiki/Packaging/PHP#Naming_scheme

- removing .php suffix from installed binary seems a good idea 

- shebang must be fixed ("/usr/bin/env php" => %{_bindir}/php)
Comment 2 Olivier BONHOMME 2011-05-14 09:58:07 EDT

Thanks for your comments. I considered all your remarks and made new spec file. You can find : 

- The SPEC file : http://ares.ptitoliv.net/~ptitoliv/fedora/smarty-gettext/smarty-gettext.spec
- The SRPM file : http://ares.ptitoliv.net/~ptitoliv/fedora/smarty-gettext/php-Smarty-gettext-1.0-0.1.b1.fc14.src.rpm

Comment 3 Remi Collet 2011-05-15 11:51:37 EDT
- spec file name must be fixed

- use install with -p option to preserve file timestamp

- sed are unreadable, use special substitution instead

- should preserve timestamp on upstream as much as possible

per exemple : 
# UNIX format conversion
for fic in ChangeLog README COPYING; do
  touch -r $fic .timestamp
  sed -i -e 's/\f//;s/\r//' $fic
  touch -r .timestamp $fic

- remove rm -rf %{buildroot} in %prep
$ rpmlint  -I rpm-buildroot-usage
$RPM_BUILD_ROOT should not be touched during %build or %prep stage, as it may
break short circuit builds.

-add rm -rf %{buildroot} in %install

- issue with license file
$ rpmlint -I incorrect-fsf-address
The Free Software Foundation address in this file seems to be outdated or
misspelled.  Ask upstream to update the address, or if this is a license file,
possibly the entire file with a new copy available from the FSF.

You should, at least, open an upstream bug (don't know if he's alive, as this 1.0b1 seems quite old... 2005)
Comment 4 Olivier BONHOMME 2011-05-15 18:06:19 EDT

Here is a new version considering the last notes : 

- SPEC file : http://ares.ptitoliv.net/~ptitoliv/fedora/smarty-gettext/php-Smarty-gettext.spec
- SRPM file : http://ares.ptitoliv.net/~ptitoliv/fedora/smarty-gettext/php-Smarty-gettext-1.0-0.1.b1.fc14.src.rpm

Comment 5 Olivier BONHOMME 2011-07-03 17:41:10 EDT

I have made a new version considering the new recommendations provided by the FC15 rpmlint software.

- SPEC file :
- SRPM file :

Comment 6 Olivier BONHOMME 2011-12-01 12:16:12 EST

I just repackaged the php-Smarty-gettext library for FC16. You'll find the elements here : 

- SPEC File : http://ares.ptitoliv.net/~ptitoliv/fedora/smarty-gettext/php-Smarty-gettext.spec
- SRPM File : http://ares.ptitoliv.net/~ptitoliv/fedora/smarty-gettext/php-Smarty-gettext-1.0-0.2.b1.fc16.src.rpm

Comment 7 Olivier BONHOMME 2012-08-23 08:56:48 EDT

Here is the new RPM for the smarty gettext library rebuilt for Fedora 17. The main change is the manpage add for the tsmarty2c script.

The ressources are here : 

- SPEC File : http://ares.ptitoliv.net/~ptitoliv/fedora/smarty-gettext/php-Smarty-gettext.spec
- SRPM File : http://ares.ptitoliv.net/~ptitoliv/fedora/smarty-gettext/php-Smarty-gettext-1.0-0.3.b1.fc17.src.rpm.

Moreover, since the project is inactive since years, it is planned to fork or recover the upstream project in order to import patches and updates.

Comment 8 Matthias Runge 2013-05-14 06:41:15 EDT
Olivier, are you still around and interested?
Comment 9 Olivier BONHOMME 2013-05-14 07:12:39 EDT
Hello Matthias,

Of course I am. The smarty-gettext lib is still used by our project Fusiondirectory which is still looking for a sponsor.

Comment 10 Matthias Runge 2013-05-14 07:17:15 EDT
OK, I'll do a review, and also could sponsor you.
Comment 11 Matthias Runge 2013-05-14 07:33:20 EDT
some drive-by comments:
- please drop buildroot definition, it's not required any more
- please drop provides and obsoletes, esp. the latter doesn't make sense at all here
- please remove the clean section, not required any more
- please update to latest release.
- please drop defattr in files section, not required any more
Comment 12 Matthias Runge 2013-08-26 04:55:22 EDT
Any progress here?
Comment 13 Matthias Runge 2013-09-24 04:38:10 EDT
Closing this. Please open another one, when you're ready.