Bug 854837 - Review Request: inkscape-sozi - Inkscape extension for creating animated presentations
Review Request: inkscape-sozi - Inkscape extension for creating animated pres...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
unspecified Severity unspecified
: ---
: ---
Assigned To: Itamar Reis Peixoto
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2012-09-06 02:28 EDT by Eduardo Echeverria
Modified: 2012-12-02 21:25 EST (History)
10 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-12-02 21:25:24 EST
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
itamar: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)
Full review file (8.13 KB, text/plain)
2012-09-06 12:18 EDT, Erik Schilling
no flags Details

  None (edit)
Description Eduardo Echeverria 2012-09-06 02:28:40 EDT
Spec URL: http://www.saef.com.ve/fedorarpm/sozi/sozi.spec
SRPM URL: http://www.saef.com.ve/fedorarpm/sozi/sozi-12.05-1.fc17.src.rpm
Summary: Inkscape extension for creating animated presentations
Description: Sozi is a small program that can play animated presentations
Fedora Account System Username:echevemaster
I have other package in review in  https://bugzilla.redhat.com/show_bug.cgi?id=854176, so I will be needing a sponsor!
########################### RPMLINT ################################
$ rpmlint -i sozi/sozi-12.05-1.fc17.src.rpm
sozi.src: W: spelling-error Summary(en_US) Inkscape -> Inks cape, Inks-cape, Capeskin
The value of this tag appears to be misspelled. Please double-check.

1 packages and 0 specfiles checked; 0 errors, 1 warnings.
########################### Koji Build #############################
http://koji.fedoraproject.org/koji/taskinfo?taskID=4459722
Comment 1 Eduardo Echeverria 2012-09-06 05:57:22 EDT
######################## Koji Build F18 ###############################
http://koji.fedoraproject.org/koji/taskinfo?taskID=4460058
######################## Koji Build F16 ###############################
http://koji.fedoraproject.org/koji/taskinfo?taskID=4460066
Comment 2 Edwind Richzendy Contreras Soto 2012-09-06 09:04:46 EDT
the package it's fine, just try use %{__mkdir} instead mkdir (cosmetic change)
Comment 3 Mohamed El Morabity 2012-09-06 09:12:48 EDT
(In reply to comment #2)
> the package it's fine, just try use %{__mkdir} instead mkdir (cosmetic
> change)
No, such macros are deprecated and shouldn't be used anymore:
    http://fedoraproject.org/wiki/Packaging:Guidelines#Macros
    https://bugzilla.redhat.com/show_bug.cgi?id=669311#c14

Eduardo: sozi is an extension for inkscape; you should then name your package "inkscape-sozi":
   http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Addon_Packages_.28General.29
Comment 4 Eduardo Echeverria 2012-09-06 11:05:03 EDT
(In reply to comment #3)
> (In reply to comment #2)
> > the package it's fine, just try use %{__mkdir} instead mkdir (cosmetic
> > change)
> No, such macros are deprecated and shouldn't be used anymore:
>     http://fedoraproject.org/wiki/Packaging:Guidelines#Macros
>     https://bugzilla.redhat.com/show_bug.cgi?id=669311#c14
> 
> Eduardo: sozi is an extension for inkscape; you should then name your
> package "inkscape-sozi":
>   
> http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Addon_Packages_.
> 28General.29

Thanks Edwin 
Thanks Mohamed  
New Spec and SRPM 
Spec URL: http://www.saef.com.ve/fedorarpm/sozi/inkscape-sozi.spec
SRPM URL: http://www.saef.com.ve/fedorarpm/sozi/inkscape-sozi-12.05-2.fc17.src.rpm
########################### RPMLINT ################################
$ rpmlint -i inkscape-sozi-12.05-2.fc17.src.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
########################## Koji Build f17 ##########################
http://koji.fedoraproject.org/koji/taskinfo?taskID=4460918
Comment 5 Luis Bazan 2012-09-06 11:08:24 EDT
Hi Edwin

must complete the %build section.

Regards!
Comment 6 Luis Bazan 2012-09-06 11:14:02 EDT
And spaces between lines changelog

:)
Comment 7 Eduardo Echeverria 2012-09-06 11:17:50 EDT
(In reply to comment #5)
> Hi Edwin
> 
> must complete the %build section.
> 
> Regards!

Hi Luis and Thanks
No need% build section, the application instructions in http://sozi.baierouge.fr/wiki/en:install only request copy files to the folder inkscape extensions
Comment 8 Eduardo Echeverria 2012-09-06 11:51:28 EDT
(In reply to comment #7)
> (In reply to comment #5)
> > Hi Edwin
> > 
> > must complete the %build section.
> > 
> > Regards!
> 
> Hi Luis and Thanks
> No need% build section, the application instructions in
> http://sozi.baierouge.fr/wiki/en:install only request copy files to the
> folder inkscape extensions

New Spec and SRPM 
Spec URL: http://www.saef.com.ve/fedorarpm/sozi/inkscape-sozi.spec
SRPM URL: http://www.saef.com.ve/fedorarpm/sozi/inkscape-sozi-12.05-3.fc17.src.rpm
Comment 9 Erik Schilling 2012-09-06 12:17:25 EDT
Hello,

I am not sponsered so this is an informal review.
If i claim anything wrong please correct me (still learning):

Problems (MUST):
[!]: MUST Changelog in prescribed format.
     One of your changelog entries has a trailing $.
     Format should be version-release.
     https://fedoraproject.org/wiki/Packaging/Guidelines#Changelogs
[!]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf is only needed if supporting EPEL5
[!]: MUST License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. No licenses
     found. Please check the source files for licenses manually.

     If the source is dual licensed use "or" instead of "and":
     https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Dual_Licensing_Scenarios
[!]: MUST If the package is under multiple licenses, the licensing breakdown
     must be documented in the spec.

Problems (SHOULD):
[!]: SHOULD SourceX / PatchY prefixed with %{name}.
     Note: Source0 (sozi-release-12.05-08120927.zip)
     Since this is the way it is released by upstream i think it is ok. But fedora-review complained about it.
[!]: SHOULD Latest version is packaged.
     According to this list: https://github.com/senshu/Sozi/downloads 12.09 is available

Questions:
[?]: MUST Package does not generate any conflict.
     Note: Package contains no Conflicts: tag(s)
     Question: Why is Provides:   %{pkgname} = %{version}-%{release} needed?
[?]: MUST If the package is a rename of another package, proper Obsoletes and
     Provides are present.
     See question above

Not checked:
[?]: MUST Package uses nothing in %doc for runtime.
[?]: SHOULD Final provides and requires are sane (rpm -q --provides and rpm -q
     --requires).
[?]: SHOULD Package functions as described.
[?]: SHOULD Package should compile and build into binary rpms on all supported
     architectures.

Rpmlint
-------
Checking: inkscape-sozi-12.05-2.fc17.src.rpm
          inkscape-sozi-12.05-2.fc17.noarch.rpm
inkscape-sozi.noarch: W: incoherent-version-in-changelog 12.05-08120927-2$ ['12.05-2.fc17', '12.05-2']
inkscape-sozi.noarch: E: script-without-shebang /usr/share/inkscape/    extensions/sozi_extras_upgrade.inx
inkscape-sozi.noarch: E: script-without-shebang /usr/share/inkscape/    extensions/sozi_extras_addvideo.js
inkscape-sozi.noarch: E: script-without-shebang /usr/share/inkscape/    extensions/sozi.inx
inkscape-sozi.noarch: E: script-without-shebang /usr/share/inkscape/    extensions/sozi_extras_addvideo_upgrade.py
inkscape-sozi.noarch: E: script-without-shebang /usr/share/inkscape/    extensions/sozi.css
inkscape-sozi.noarch: E: script-without-shebang /usr/share/inkscape/    extensions/sozi_extras_addvideo.inx
inkscape-sozi.noarch: E: script-without-shebang /usr/share/inkscape/    extensions/sozi_upgrade.py
inkscape-sozi.noarch: E: script-without-shebang /usr/share/inkscape/    extensions/sozi.js
2 packages and 0 specfiles checked; 8 errors, 1 warnings.

I doubt that these files are intended to be runnable / executable. They do not look executable to me. So i do not know why they show up.
Can sombody more expirienced clear me up?


MD5-sum check
-------------
https://github.com/downloads/senshu/Sozi/sozi-release-12.05-08120927.   zip :
  CHECKSUM(SHA256) this package     :                                   55b0e1c0351feb3cd6f28e72019f3f93aa02d609cabc7b96b749914bc2b1c24e
  CHECKSUM(SHA256) upstream package :                                   55b0e1c0351feb3cd6f28e72019f3f93aa02d609cabc7b96b749914bc2b1c24e

Full review file is uploaded.

Best regards,
Erik Schilling
Comment 10 Erik Schilling 2012-09-06 12:18:02 EDT
Created attachment 610409 [details]
Full review file
Comment 11 Eduardo Echeverria 2012-09-06 13:46:32 EDT


(In reply to comment #10)
> Created attachment 610409 [details]
> Full review file

Thanks Erik 

The version 12.09 is not stable even https://groups.google.com/forum/?fromgroups = #! Topic/sozi-users/K33c12EcQqM

The license file is located in:
%files
%doc install-en.html GPL-license.txt install-fr.html MIT-license.txt

Other changes based on your comments in:

New Spec and SRPM 
Spec URL: http://www.saef.com.ve/fedorarpm/sozi/inkscape-sozi.spec
SRPM URL: http://www.saef.com.ve/fedorarpm/sozi/inkscape-sozi-12.05-4.fc17.src.rpm
Comment 12 Eduardo Echeverria 2012-09-08 05:15:41 EDT
New Spec and SRPM 
Spec URL: http://www.saef.com.ve/fedorarpm/sozi/5/inkscape-sozi.spec
SRPM URL: http://www.saef.com.ve/fedorarpm/sozi/5/inkscape-sozi-12.05-5.fc17.src.rpm

#####################Koji Build f17 ##############################
http://koji.fedoraproject.org/koji/taskinfo?taskID=4466933
#################### Koji Build f18 ##############################
http://koji.fedoraproject.org/koji/taskinfo?taskID=4466935
############################ rpmlint SRPM ########################
$ rpmlint -i inkscape-sozi-12.05-5.fc17.src.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
############################ rpmlint RPM ########################
$ rpmlint -i rpmlint -i inkscape-sozi-12.05-5.fc17.noarch.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
Comment 13 Toshio Ernie Kuratomi 2012-09-08 11:11:38 EDT
itamarjp has agreed to mentor you.  I will sponsor you into the packager group and itamar will be your primary contact for help learning how to package for Fedora.  You can also contact me (abadger1999 on irc) if there's any questions that itamar cannot answer.
Comment 14 Eduardo Echeverria 2012-09-08 15:49:29 EDT
Thank you very much Toshio and Itamar, in waiting for your valuable comments
regards
Comment 15 Itamar Reis Peixoto 2012-09-10 21:01:21 EDT
Eduardo, please look at koji build logs.

warning: File listed twice: /usr/share/inkscape/extensions/sozi.css
...
...
warning: File listed twice: /usr/share/inkscape/extensions/sozi_upgrade.pyo

adding the flowing line into %files section.

%{_datadir}/inkscape/extensions/sozi*

will include all files starting with sozi.

when you list the files starting with sozi again, gives the warning.

%attr(644,root,root) %{_datadir}/inkscape/extensions/sozi_extras_upgrade.inx*
%attr(644,root,root) %{_datadir}/inkscape/extensions/sozi_extras_addvideo.js*
%attr(644,root,root) %{_datadir}/inkscape/extensions/sozi.inx*
%attr(644,root,root) %{_datadir}/inkscape/extensions/sozi_extras_addvideo_upgrade.py*
%attr(644,root,root) %{_datadir}/inkscape/extensions/sozi.css*
%attr(644,root,root) %{_datadir}/inkscape/extensions/sozi_extras_addvideo.inx*
%attr(644,root,root) %{_datadir}/inkscape/extensions/sozi_upgrade.py*
%attr(644,root,root) %{_datadir}/inkscape/extensions/sozi.js*

look -> 

http://fedoraproject.org/wiki/How_to_create_an_RPM_package#.25files_section

%defattr(<file permissions>, <user>, <group>, <directory permissions>)


what do you think about cleaning your %files section to something like this ->

%files
%defattr(0644,root,root,-)
%doc install-en.html GPL-license.txt install-fr.html MIT-license.txt
%{_datadir}/inkscape/extensions/sozi*

I think will produce same result, with installed files set as 0644, and no warning about files listed twice in build log.
Comment 16 Eduardo Echeverria 2012-09-11 00:17:54 EDT
Changes Ready Itamar: 

http://echevemaster.fedorapeople.org/inkscape-sozi/inkscape-sozi-12.05-6.fc17.src.rpm
http://echevemaster.fedorapeople.org/inkscape-sozi/inkscape-sozi.spec

$ rpmlint -i inkscape-sozi-12.05-6.fc17.noarch.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings

$ rpmlint -i inkscape-sozi-12.05-6.fc17.src.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
Comment 17 Eduardo Echeverria 2012-09-11 00:40:15 EDT
http://echevemaster.fedorapeople.org/inkscape-sozi/7/inkscape-sozi-12.05-7.fc17.src.rpm
http://echevemaster.fedorapeople.org/inkscape-sozi/7/inkscape-sozi.spec

$ rpmlint -i inkscape-sozi-12.05-7.fc17.noarch.rpm 
1 packages and 0 specfiles checked; 0 errors, 0 warnings

$ rpmlint -i inkscape-sozi-12.05-7.fc17.src.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
Comment 18 Itamar Reis Peixoto 2012-09-11 23:34:54 EDT
Package Review
==============

Key:
- = N/A
x = Pass
! = Fail
? = Not evaluated



==== Generic ====
[x]: EXTRA Rpmlint is run on all installed packages.
[x]: EXTRA Spec file according to URL is the same as in SRPM.
[x]: MUST Package is licensed with an open-source compatible license and meets
     other legal requirements as defined in the legal section of Packaging
     Guidelines.
[x]: MUST Package successfully compiles and builds into binary rpms on at
     least one supported primary architecture.
[x]: MUST %build honors applicable compiler flags or justifies otherwise.
[x]: MUST All build dependencies are listed in BuildRequires, except for any
     that are listed in the exceptions section of Packaging Guidelines.
[x]: MUST Package contains no bundled libraries.
[x]: MUST Changelog in prescribed format.
[x]: MUST Sources contain only permissible code or content.
[x]: MUST Each %files section contains %defattr if rpm < 4.4
     Note: defattr macros not found. They would be needed for EPEL5
[x]: MUST Macros in Summary, %description expandable at SRPM build time.
[-]: MUST Package contains desktop file if it is a GUI application.
[-]: MUST Development files must be in a -devel package
[x]: MUST Package requires other packages for directories it uses.
[x]: MUST Package uses nothing in %doc for runtime.
[x]: MUST Package is not known to require ExcludeArch.
[x]: MUST Permissions on files are set properly.
[x]: MUST Package does not contain duplicates in %files.
[x]: MUST Package complies to the Packaging Guidelines
[x]: MUST Spec file lacks Packager, Vendor, PreReq tags.
[x]: MUST Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
     beginning of %install.
     Note: rm -rf would be needed if support for EPEL5 is required
[-]: MUST Large documentation files are in a -doc subpackage, if required.
[x]: MUST If (and only if) the source package includes the text of the
     license(s) in its own file, then that file, containing the text of the
     license(s) for the package is included in %doc.
[x]: MUST License field in the package spec file matches the actual license.
     Note: Checking patched sources after %prep for licenses. No licenses
     found. Please check the source files for licenses manually.
[x]: MUST Package consistently uses macro is (instead of hard-coded directory
     names).
[x]: MUST Package is named using only allowed ascii characters.
[x]: MUST Package is named according to the Package Naming Guidelines.
[x]: MUST Package does not generate any conflict.
     Note: Package contains no Conflicts: tag(s)
[x]: MUST Package obeys FHS, except libexecdir and /usr/target.
[x]: MUST If the package is a rename of another package, proper Obsoletes and
     Provides are present.
[x]: MUST Package must own all directories that it creates.
[x]: MUST Package does not own files or directories owned by other packages.
[x]: MUST Package installs properly.
[x]: MUST Package is not relocatable.
[x]: MUST Requires correct, justified where necessary.
[x]: MUST Rpmlint is run on all rpms the build produces.
     Note: No rpmlint messages.
[x]: MUST Sources used to build the package match the upstream source, as
     provided in the spec URL.
[x]: MUST Spec file is legible and written in American English.
[x]: MUST Spec file name must match the spec package %{name}, in the format
     %{name}.spec.
[-]: MUST Package contains systemd file(s) if in need.
[x]: MUST File names are valid UTF-8.
[x]: SHOULD Reviewer should test that the package builds in mock.
[x]: SHOULD Buildroot is not present
     Note: Unless packager wants to package for EPEL5 this is fine
[x]: SHOULD Package has no %clean section with rm -rf %{buildroot} (or
     $RPM_BUILD_ROOT)
     Note: Clean would be needed if support for EPEL5 is required
[x]: SHOULD If the source package does not include license text(s) as a
     separate file from upstream, the packager SHOULD query upstream to
     include it.
[x]: SHOULD Dist tag is present.
[x]: SHOULD No file requires outside of /etc, /bin, /sbin, /usr/bin,
     /usr/sbin.
[x]: SHOULD Final provides and requires are sane (rpm -q --provides and rpm -q --requires).
[?]: SHOULD Package functions as described.
[!]: SHOULD Latest version is packaged.
	this is the last officially published in upstream website.
[x]: SHOULD Package does not include license text files separate from
     upstream.
[x]: SHOULD Patches link to upstream bugs/comments/lists or are otherwise
     justified.
[x]: SHOULD SourceX tarball generation or download is documented.
[-]: SHOULD SourceX / PatchY prefixed with %{name}.
[x]: SHOULD SourceX is a working URL.
[-]: SHOULD Description and summary sections in the package spec file contains
     translations for supported Non-English languages, if available.
[x]: SHOULD Package should compile and build into binary rpms on all supported
     architectures.
[-]: SHOULD %check is present and all tests pass.
[x]: SHOULD Packages should try to preserve timestamps of original installed
     files.
[x]: SHOULD Spec use %global instead of %define.



Rpmlint
-------
[itamar@localhost sozi]$ rpmlint inkscape-sozi-12.05-7.fc17.src.rpm /home/itamar/rpmbuild/RPMS/noarch/inkscape-sozi-12.05-7.fc17.noarch.rpm  
2 packages and 0 specfiles checked; 0 errors, 0 warnings.

Requires
--------
[itamar@localhost sozi]$ rpm -q --requires inkscape-sozi
/usr/bin/env  
inkscape  
pygtk2  
python-lxml  
python2  
rpmlib(CompressedFileNames) <= 3.0.4-1
rpmlib(FileDigests) <= 4.6.0-1
rpmlib(PartialHardlinkSets) <= 4.0.4-1
rpmlib(PayloadFilesHavePrefix) <= 4.0-1
rpmlib(PayloadIsXz) <= 5.2-1

Provides
--------
[itamar@localhost sozi]$ rpm -q --provides inkscape-sozi
inkscape-sozi = 12.05-7.fc17


sha1sum check
-------------
81bb720147892690539e567e6937cb21b9e09e85  http://cloud.github.com/downloads/senshu/Sozi/sozi-release-12.05-08120927.zip
81bb720147892690539e567e6937cb21b9e09e85  sozi-release-12.05-08120927.zip in src.rpm

Package APPROVED


please follow this package and request a git repository.

http://fedoraproject.org/wiki/Package_SCM_admin_requests#New_Packages
Comment 19 Eduardo Echeverria 2012-09-12 00:32:30 EDT
New Package SCM Request
=======================
Package Name: inkscape-sozi
Short Description: Inkscape extension for creating animated presentations
Owners: echevemaster
Branches: f17 f18 devel
InitialCC: toshio itamarjp
Comment 20 Gwyn Ciesla 2012-09-12 07:05:56 EDT
Git done (by process-git-requests).
Comment 21 Fedora Update System 2012-09-12 13:20:33 EDT
inkscape-sozi-12.05-7.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/inkscape-sozi-12.05-7.fc17
Comment 22 Fedora Update System 2012-09-12 13:21:52 EDT
inkscape-sozi-12.05-7.fc18 has been submitted as an update for Fedora 18.
https://admin.fedoraproject.org/updates/inkscape-sozi-12.05-7.fc18
Comment 23 Fedora Update System 2012-09-17 18:06:36 EDT
inkscape-sozi-12.05-7.fc18 has been pushed to the Fedora 18 stable repository.
Comment 24 Fedora Update System 2012-09-26 04:50:53 EDT
inkscape-sozi-12.05-7.fc17 has been pushed to the Fedora 17 stable repository.

Note You need to log in before you can comment on or make changes to this bug.