Description of problem: SRPM URL: http://orion.lcg.ufrj.br/RPMS/src/xpaint-2.8.2-5.fc10.src.rpm Spec URL: http://orion.lcg.ufrj.br/RPMS/SPECS/xpaint.spec XPaint is an X Window System color image bitmap editing program. It does not try to compete with more advanced tools such as 'gimp' but aims at being very easy to use. XPaint however supports most standard paint program operations. It also supports more advanced features, such as image processing algorithms and scripting. XPaint allows the edition of multiple images simultaneously and supports a wide variety of image formats, including: GIF, JPG, PNG, PPM, TIFF, XBM, XPM, etc. Install xpaint if you need an easy to use paint program for X. Xpaint now uses the Xaw3d widget set for a bit nicer look. The package includes several examples of C scripts: filters, image scripts adding some new editing features including user filters. Some example filter code is included. rpmlint is clean. need to recheck license. Version-Release number of selected component (if applicable): How reproducible: Steps to Reproduce: 1. 2. 3. Actual results: Expected results: Additional info:
A couple of notes: - URL should be http://sf-xpaint.sourceforge.net/ - Source0 should be http://downloads.sourceforge.net/sf-xpaint/%{name}-%{version}.tar.bz2 http://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net
Fixed.Thanks.
Ok, i cant get the source from that URL in the spec, 404 is given. There's not even a downloadlink on the page that comes up with the URL: from the spec. A page with a downloadlink comes up with: https://sourceforge.net/projects/sf-xpaint A working downloadlink is: http://downloads.sourceforge.net/project/sf-xpaint/sf-xpaint/%{name}-%{version}/%{name}-%{version}.tar.bz2 MUST: rpmlint must be run on every package. The output should be posted in the review. Where did you find the MIT license? There's no license file in the tarball and the project homepage says: License: "Other license" which is a link to a page with other projects. MUST: The package must be licensed with a Fedora approved license and meet the http://fedoraproject.org/wiki/Packaging/LicensingGuidelines MUST: The License field in the package spec file must match the actual license. Upstream has released 2.8.3 by the way. MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture. Can you post the 'Task Info' link to a koji scratch build here, thanks. Icon tag in Desktop Files, the icon tag can be specified in two ways: Full path to specific icon file or shortname without file extension. You use shortname with file extension. Missing: BuildRequires: desktop-file-utils desktop-file-install MUST be used if the package does not install the file or there are changes desired to the .desktop file (such as add/removing categories, etc). http://fedoraproject.org/wiki/Packaging/Guidelines#desktop -- Fedora Bugzappers volunteer triage team https://fedoraproject.org/wiki/BugZappers
If this is your first package (i cant find your email in FAS) you need to: add FE-NEEDSPONSOR to the bugs being blocked by your review request. There's also: https://bugzilla.redhat.com/bugzilla/enter_bug.cgi?product=Fedora&format=extras-review for reviews. Maybe next time ;) I added NotReady to the whiteboard due to the license question. Clear the whiteboard when it's fixed. -- Fedora Bugzappers volunteer triage team https://fedoraproject.org/wiki/BugZappers
I fixed the spec, but still have to check the license: SRPM URL: http://orion.lcg.ufrj.br/RPMS/src/xpaint-2.8.3-5.fc10.src.rpm Spec URL: http://orion.lcg.ufrj.br/RPMS/SPECS/xpaint.spec
In the source there is a file debian/copyright: --------------------------------------------------------- This package was debianized by Mark W. Eichin eichin.ma.us on Wed, 26 Feb 1997 12:37:58 -0500. The current Debian maintainer is Hugo Vanwoerkom <hugovanwoerkom>. The sourcecode can be downloaded from https://sourceforge.net/projects/sf-xpaint/ Note that parts of this are GPL and parts are redistributable as-is. Copyright: Copyright (C) 1990, 1991, 1992, 1993, David Koblas Copyright (C) 1995, 1996, 1997, 1998, Torsten Martinsen Copyright (C) 1996-2003, Greg Roelofs Copyright (C) 1997, Scott D. Nelson Copyright (C) 1995, Tor Lillqvist ............ It is not clear for me, reading the whole file, if I can use just GPLv2 or stick with MIT: The link for the debian, ubuntu, and alt linux packages are these: http://packages.debian.org/unstable/graphics/xpaint http://packages.ubuntu.com/karmic/xpaint http://sisyphus.ru/en/srpm/xpaint Any clue here?
Yes, if in doubt, and that's the case here, ask upstream. Would be perfect if they provide a proper LICENSE file. You could add that then with sourceX. -- Fedora Bugzappers volunteer triage team https://fedoraproject.org/wiki/BugZappers
(In reply to comment #7) > Yes, if in doubt, and that's the case here, ask upstream. Would be perfect if > they provide a proper LICENSE file. > > You could add that then with sourceX. > > This is the answer to my consult: > > I would like to include xpaint in Fedora, but it is not > clear for me > what is its license., even reading the file debian/copyright. > > Would it be possible to have an official license file in the > project? > Hi: I am sorry that I cannot really clarify the issue. Xpaint has a long history and combined different parts with different licences, including MIT and GPL. Everything seems to be GPL compatible and I consider all my additions to be GPL whenever this can apply - I don't think there is any issue, but I am not an expert in these legalities... In any case I'll add the text of GPLv2 in the next release and I encourage you to do so for the Fedora package if you feel this is desirable. Best, Jean-Pierre Demailly
Sounds not too good. I will have fedora-legal look over it. Sorry. -- Fedora Bugzappers volunteer triage team https://fedoraproject.org/wiki/BugZappers
(In reply to comment #9) > Sounds not too good. I will have fedora-legal look over it. Sorry. Here it is stated that MIT license is compatible with both GPLv2 and GPLv3: https://fedoraproject.org/wiki/Licensing:Main#Good_Licenses This means that you can combine code released under the MIT license with code released under the GNU GPL in one larger program: http://www.gnu.org/licenses/gpl-faq.html#WhatDoesCompatMean HTH, Andrea
I had a glance about the package. This is what else I noticed. * rpmlint /var/lib/mock/fedora-11-i386/result/*.rpm xpaint.src:246: W: macro-in-%changelog config xpaint-devel.i586: W: no-documentation 4 packages and 0 specfiles checked; 0 errors, 2 warnings. There are two occurences of % that haven't been escaped in the %changelog. Please fix them. * Missing SMP flags. If it doesn't build with it, please add a comment (wiki: Packaging/Guidelines#parallelmake) * Duplicate BuildRequires: zlib-devel (by libpng-devel), libXpm-devel (by libXaw-devel), xorg-x11-proto-devel (by libXaw-devel), libSM-devel (by Xaw3d-devel), libXext-devel (by Xaw3d-devel) * Not end user documentation files installed: /usr/share/doc/xpaint-2.8.2/INSTALL Install instruction must not be installed /usr/share/doc/xpaint-2.8.2/Doc/CHANGES This changelog is so old that I really doubt it is useful /usr/share/doc/xpaint-2.8.2/Doc/Imakefile /usr/share/doc/xpaint-2.8.2/Doc/Makefile /usr/share/doc/xpaint-2.8.2/Doc/Makefile.bak 3 not documentation files. /usr/share/doc/xpaint-2.8.2/Doc/Operator.doc Developer file. /usr/share/doc/xpaint-2.8.2/Doc/sample.Xdefaults Maybe this is useful but because it will be the only useful file in this directory I think it will be better to move it to the parent dir. * Probably it is a good idea to include README.old among docs. Debian does this too. * There is no suggest in RPM therefore I think you should require gv, netpbm, lpr, xv, emacs Or otherwise make a README.Fedora file where you state what other program can be installed and why. "In the C Script Editor of XPaint, the External editor in the File menu will invoke <program_name>. The gs package will be needed for PDF/PS reading, netpbm will be needed for external conversion, lpr for printing. <ETC....> " * In the desktop file: Categories=Application;Graphics; is not good. Application is not a registered category: http://standards.freedesktop.org/menu-spec/latest/apa.html Please use: Graphics;2DGraphics;RasterGraphics; * Please use Mandriva description because it is more accurate (and based on author feedback too) http://svn.mandriva.com/cgi-bin/viewvc.cgi/packages/cooker/xpaint/current/SPECS/xpaint.spec?revision=452128&view=markup * Devel package is probably not needed and wrong. Xpaint uses some very strange C scripting. Without files you package in devel it cannot work. BTW, both Debian and Mandriva do not have a devel package. "The C script Editor can be used to write a script in language C, defining e.g. a programmable filter. This possibility will appeal to programmers who want to design their own filter routines. The "Compile" button compiles the C script as a shared ???.so library, and, if successful, links dynamically that library to Xpaint. Several examples will be found in the share/filters subdirectory. Notice that you can use this feature only if gcc has been installed on your system." * The following lines seem unneeded because both variables should be correctly set when invoking make. perl -p -i -e "s|CXXDEBUGFLAGS = .*|CXXDEBUGFLAGS = $RPM_OPT_FLAGS|" Makefile perl -p -i -e "s|CDEBUGFLAGS = .*|CDEBUGFLAGS = $RPM_OPT_FLAGS|" Makefile * You should probably patch Local.config to use "better" Fedora programs. This is what you can find there: EDITOR = emacs -fn 9x15 -cr green -ms red -bg lightyellow -fg black POSTSCRIPT_VIEWER = gv EXTERN_VIEWER = xv
(In reply to comment #10) > (In reply to comment #9) > > Sounds not too good. I will have fedora-legal look over it. Sorry. > > Here it is stated that MIT license is compatible with both GPLv2 and GPLv3: > https://fedoraproject.org/wiki/Licensing:Main#Good_Licenses > > This means that you can combine code released under the MIT license with code > released under the GNU GPL in one larger program: > http://www.gnu.org/licenses/gpl-faq.html#WhatDoesCompatMean Yes, that's true. My point is "different parts with different licences, *including* MIT and GPL". Including MIT and GPL means there are others. As long as we dont know *what* licenses the others are, it's a no-go from my understanding. Upstream could be bothered to find out. On the plus side would be that it's included in Debian. But i have no idea what "debianized" means. He could ask the Debian developer to find out. Sorry. -- Fedora Bugzappers volunteer triage team https://fedoraproject.org/wiki/BugZappers
(In reply to comment #12) > Yes, that's true. My point is "different parts with different licences, > *including* MIT and GPL". Including MIT and GPL means there are others. As long > as we dont know *what* licenses the others are, it's a no-go from my > understanding. Ops... sorry for my misunderstanding. > Upstream could be bothered to find out. On the plus side would be that it's > included in Debian. But i have no idea what "debianized" means. He could ask > the Debian developer to find out. +1. I agree that asking Debian package maintainer is the way to go. Debian is usually very careful about license issues. PS "Debianized" just means that someone has make a Debian .deb package.
SRPM URL: http://orion.lcg.ufrj.br/RPMS/src/xpaint-2.8.3-6.fc10.src.rpm Spec URL: http://orion.lcg.ufrj.br/RPMS/SPECS/xpaint.spec
http://koji.fedoraproject.org/koji/taskinfo?taskID=1739238 I will check upstream about the license.
Version 2.8.4 was just release under GPLv3: http://orion.lcg.ufrj.br/RPMS/src/xpaint-2.8.4-7.fc10.src.rpm http://orion.lcg.ufrj.br/RPMS/SPECS/xpaint.spec
I want to give that package-review to Andrea Musuruane. He has done the bigger part of the review. -- Fedora Bugzappers volunteer triage team https://fedoraproject.org/wiki/BugZappers
Just a few comments (I'm no sponsor anyway ;)): rpmlint output is not clean. - Take a look to https://fedoraproject.org/wiki/Packaging/Guidelines#Beware_of_Rpath for the error with rpath - use %configure instead just ./configure - Doesn't the folder /usr/share/xpaint/include/ belongs to /usr/include/xpaint? At least this folder should go into a -devel subpackage. What about the c_scripts folder? It it needed at runtime? - There is a new version upstream (again^^).
(In reply to comment #18) > - Doesn't the folder /usr/share/xpaint/include/ belongs to /usr/include/xpaint? > At least this folder should go into a -devel subpackage. > What about the c_scripts folder? It it needed at runtime? Please read previous comments about this. Andrea.
(In reply to comment #17) > I want to give that package-review to Andrea Musuruane. He has done the bigger > part of the review. Thomas, if you want to go on, you have my blessing. I have little time now to do this review. Andrea.
(In reply to comment #18) > Just a few comments (I'm no sponsor anyway ;)): I do not need a sponsor. http://koji.fedoraproject.org/koji/userinfo?userID=866 > > rpmlint output is not clean. > > - Take a look to > https://fedoraproject.org/wiki/Packaging/Guidelines#Beware_of_Rpath for the > error with rpath > - use %configure instead just ./configure > > - Doesn't the folder /usr/share/xpaint/include/ belongs to /usr/include/xpaint? > At least this folder should go into a -devel subpackage. > What about the c_scripts folder? It it needed at runtime? > The question is whether we need a devel package or not. Originally, all .c and .h were in a devel subpackage. However, xpaint allows one to write plugins in C, and Andrea suggested the suppression of the devel package, as I understood. I will take a look at version 2.8.5 Thanks.
New SRPM URL: http://orion.lcg.ufrj.br/RPMS/src/xpaint-2.8.5-8.fc10.src.rpm
This does not build in mock because you are running chrpath, but the chrpath package is no BuildRequires, so it's not installed. Did you try to remove the rpath with sed after configure? chrpath should only be used as last resort.
BTW: The release is wrong: You increased the version, so the Release starts with 1 again, not 8.
(In reply to comment #23) > This does not build in mock because you are running chrpath, but the chrpath > package is no BuildRequires, so it's not installed. > > Did you try to remove the rpath with sed after configure? chrpath should only > be used as last resort. Yes I tried. The problem is that xpaint uses "xmkmf". I have uploaded the wrong .src.rpm, and that is why chrpath did not appear as a BR. Can you download it again? Same link.
The link cannot be correct because the package name needs to change: http://orion.lcg.ufrj.br/RPMS/src/xpaint-2.8.5-1.fc10.src.rpm instead of ^ http://orion.lcg.ufrj.br/RPMS/src/xpaint-2.8.5-8.fc10.src.rpm
Here it is: http://orion.lcg.ufrj.br/RPMS/src/xpaint-2.8.5-1.fc10.src.rpm
(In reply to comment #21) > I do not need a sponsor. Sorry, I read something about it in comment #4 and found no answer to it in several comments down (or STRG+f didn't find it for me ;)). I hugely dislike the way this program handles includes and so on and therefor don't want to review this… I hope there is a way to patch this..... You could make a devel package anyway and the normal package could require it always. This way rpmlint would be cleaner and the devel package can be noarch, because it's just source code and save a bit space across arches, but for 172K I doubt, this beeing much better^^ Judging from [1] it should always be worth splitting. [1]https://fedoraproject.org/wiki/Features/NoarchSubpackages
xpaint can be run without any included "C code". However, a user can call the C script editor from the tab options, and then load and compile from the template tab, a filter, a surface, etc... Rpmlint gives us some guidelines, but there are several Fedora packages which produce dozen of warnings. I personally do not care if xpaint has a devel package or not. It is really better a single package with everything inside, though. Our role as packagers is secondary. We just need to ship a useful system. If we dislike some upstream decisions based on our packaging standards is not the main point, in my opinion. Xpaint is an old program, which was part of RedHat in the past. However, it has made some improvements along the time. According to the current developer: "Versions >= 2.8.4 include a new high performance postscript generator. The PS files are e.g. often less than 50% the size of the (already compressed) PS files that gimp produces, and the new thing is very fast - see enclosed 'ppmtops' prototype. I don't think there are so many open source programs, even among the very established ones, that are "aware" enough of these algorithms; of course 'ppmtops' relies on a combination of PNG predictors with LZW compression which had been patented till around 2004, so maybe it's the reason - if Adobe would provide better support for unpatented compression schemes as an alternative to LZW, one could do even better."
SRPM URL: http://orion.lcg.ufrj.br/RPMS/src/xpaint-2.8.6.1-1.fc10.src.rpm SPEC: http://orion.lcg.ufrj.br/RPMS/SPECS/xpaint.spec Created a devel subpackage, which is noarch: [lua:~/specs] rpmlint xpaint xpaint.x86_64: E: devel-dependency xpaint-devel 1 packages and 0 specfiles checked; 1 errors, 0 warnings. [lua:~/specs] rpmlint xpaint-devel xpaint-devel.noarch: W: no-documentation 1 packages and 0 specfiles checked; 0 errors, 1 warnings. Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=1772200 I am just curious whether noarch subpackages will really become common or not.
By the way, all .h files are now in: /usr/include/xpaint
Another possibility with c_scripts in the main package, since they are not real devel files (but rpmlint spits some warnings): SRPM URL:http://orion.lcg.ufrj.br/RPMS/src/xpaint-2.8.6.1-2.fc10.src.rpm I think it is easy to conclude the review now, since there is no much room for new changes. Thanks.
Hmm, a few issues on a full review. One, they didn't update the GPL licensing text in all places, but that's OK since the GPLv2 stuff says GPLv2+. Given that, might want to change license tage to GPLv3+. Source URL should be Source0: http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.gz, modified for the project name. Why is the URL tag pointing to the SF project, and not the SF web page? Mock build and BRs are OK. I've also read this entire review and considered the issues raised and their solutions. I think once the various tags above are fixed, I could approve.
(In reply to comment #33) > Hmm, a few issues on a full review. > > One, they didn't update the GPL licensing text in all places, but that's OK > since the GPLv2 stuff says GPLv2+. Given that, might want to change license > tage to GPLv3+. Done, and there is a new version 2.8.7 available. > > Source URL should be Source0: > http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.gz, modified > for the project name. http://downloads.sourceforge.net/sf-xpaint/xpaint-2.8.7.tar.gz redirects to: http://sourceforge.net/projects/sf-xpaint/files/ > > Why is the URL tag pointing to the SF project, and not the SF web page? Because the SF web page does not have any downloadable link, as you can see: http://sf-xpaint.sourceforge.net/ It is kind of unusable, IMHO. > > Mock build and BRs are OK. > > I've also read this entire review and considered the issues raised and their > solutions. > > I think once the various tags above are fixed, I could approve. SPEC: http://orion.lcg.ufrj.br/RPMS/SPECS/xpaint.spec SRPM: http://orion.lcg.ufrj.br/RPMS/src/xpaint-2.8.7-1.fc10.src.rpm
Sounds reasonable re: URL. Re: Source0. Yes, that's what it redirects to. Today. We use the one in the guidelines in case it eventually redirects elsewhere, to prevent breakage. Switch, and we're good.
Done. In fact, I posted the wrong link. This is the right one: http://downloads.sourceforge.net/sf-xpaint/xpaint-2.8.7.tar.bz2 Same URLS, which are already updated.
Looks good, APPROVED.
Thanks Jon, for finishing this review, and Hans de Goede for all of his suggestions for improving this packages. New Package CVS Request ======================= Package Name: xpaint Short Description: An X Window System image editing or paint program Owners: roma Branches: F-10 F-11 F-12 InitialCC: roma
cvs done.
Jason, the package is mine in Fedora. Thanks. Package Change Request ====================== Package Name: xpaint New Branches: el5 el6 Owners: roma
Git done (by process-git-requests).