Spec URL: http://www.tenbrink-bekkers.nl/fedora/picturetile.spec SRPM URL: http://www.tenbrink-bekkers.nl/fedora/picturetile-20050314-1.fc10.src.rpm Introduction: Users need this perl script in order to use the Create Photowall option (provided by extension PictureTile 0.5.0.0 in F-Spot). The script itself is not packaged anywhere in Fedora until now. When user click Create Photowall in F-Spot, only the message "picturetile.pl not found" appears, which is not very user-friendly. Unfortunately, F-Spot has a problem with its extensions at the moment, therefore live testing of the interaction with F-Spot is at the moment impossible. I already filed a bug upstream, and add I will add a tracker bug here in Bugzilla as well. Since it is not required for F-Spot to have this package or vice versa, I deliberately left out these dependencies. This is my second package under review, but I have no sponsor up to now. My first package is under review under bug 497525. Description of the script: Takes a bunch of images and tiles them into one large image. Not so much a collage, as a "photo wall." The images are selected randomly from the given directory, and packed together as tightly as possible. Images that are very small are assumed to be thumbnails and are ignored. No image will be used more than once, and the images need not be of the same size or aspect ratio. This script can be used stand-alone from the command-line, or from the F-Spot plugin "create photowall" (PictureTile).
(In reply to comment #0) > This is my second package under review, but I have no sponsor up to now. In the meantime, my first package has been approved. I'm now sponsored by Christoph Wickert. Additionally, due to a server restructuring, the URL's have changed (files are still identical): Spec URL: http://fedora.tenbrink-bekkers.nl/picturetile/picturetile.spec SRPM URL: http://fedora.tenbrink-bekkers.nl/picturetile/picturetile-20050314-1.fc10.src.rpm
Just a few comments, not a review: This bug should block your tracker bug and not depend on it. The tracker bug can only be closed, if all bugs 'blocking' the tracker bugs are closed. If they depend all on the tracker, they all can only be closed, if the tracker gets closed first. Are you sure about the version generation? See [1]. So it should have the name picturetile-0-1.20050314.fc10.src.rpm, isn't it? I'm not sure about this, too, but simply using the date as a version number 'looks bad'.^^ e.g. from the guidelines: kismet-0-0.1.20040110svn (this is a pre-release, svn checkout of kismet) kismet-0-0.2.20040110svn (this is a bugfix to the previous package) kismet-0-0.3.20040204svn (this is a new svn checkout, note the increment of %{X}) kismet-1.0-1 (this is the formal release of kismet 1.0) Of course, there will maybe never been any real version, but how about using always version 0 and append the date after the release number? [1] https://fedoraproject.org/wiki/Packaging:NamingGuidelines#NonNumericRelease
(In reply to comment #2) > Just a few comments, not a review: > > This bug should block your tracker bug and not depend on it. I think what Edwin wants to point out it that first the bug with fspot should be fixed before he will maintain this package. Edwin, am I correct? > Are you sure about the version generation? > See [1]. So it should have the name picturetile-0-1.20050314.fc10.src.rpm, > isn't it? > I'm not sure about this, too, but simply using the date as a version number > 'looks bad'.^^ ... > [1] https://fedoraproject.org/wiki/Packaging:NamingGuidelines#NonNumericRelease This applies to numeric releases with characters such as "RC", "BETA" and so on. The date *is* numeric. As time moves forward *hint* *hint* the version will always increase. So using the date is ok. However prefixing it with a 0. is not a bad idea. Otherwise we might get into trouble if upstream one day decides to switch to another versioning.
First of all, Thomas, Christoph: thanks for your comments. I was unfortunately not available the last few days to get back to you earlier. (In reply to comment #3) > (In reply to comment #2) > > This bug should block your tracker bug and not depend on it. > > I think what Edwin wants to point out it that first the bug with fspot should > be fixed before he will maintain this package. Edwin, am I correct? Christoph is entirely correct. Since it will only be possible to effectively test the package functionality on the command-line only, and this package is meant to extend the functionality of an add-on of fspot, I believe it will only make sense if the fspot add-on actually functions. Recently, fspot has released a new version, as well as the add-on. For some reason I have not found out yet, it does no longer crash fspot, but does not seem to generate the desired output. I'm still working on this, therefore I have not yet closed bug 498222 and the dependency. > > Are you sure about the version generation? > This applies to numeric releases with characters such as "RC", "BETA" and so > on. The date *is* numeric. As time moves forward *hint* *hint* the version will > always increase. So using the date is ok. > > However prefixing it with a 0. is not a bad idea. Otherwise we might get into > trouble if upstream one day decides to switch to another versioning. I agree, an oversight on my part. I will incorporate it in my next submission. I'll get back to this as soon as the combination fspot / addon / picturetile seems to be functioning.
The add-in PictureTile version 0.6.0.1 for f-spot-0.6.1.2-3.fc11.i586 works with the package under review (picturetile-0.20050314-2). I incorporated the version-comment, so the version is now prefixed with "0." The package is otherwise unchanged. Uploaded files: Spec URL: http://fedora.tenbrink-bekkers.nl/picturetile/picturetile.spec SRPM URL: http://fedora.tenbrink-bekkers.nl/picturetile/picturetile-0.20050314-2.fc11.src.rpm
Thanks for packaging this nice small handy script. I made the full review on this. The *'s are the only blockers. The rest is remarks and questions: - rpmlint is silent * The license is MIT https://fedoraproject.org/wiki/Licensing/MIT old style. One of the oldest software licenses out there. ! Guidelines say: "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 must be included in %doc." in http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text" But this package does not even have a tarball. Is there any other examples of Fedora packages that just cut the license text from the soure file? If not, I would say it is best to obey the guideline. ? Shall we rename the executable to just picturetile? That is the upstream project name. ! It would be better to install the executable with "install -pm 755 ..." and not use %attr(0755,root,root) to comply with general Fedora conventions.. ? I am a bit reluctant about the provides: Provides: picturetile.pl = 20050314 Do we really need this? ? I would go with the suggestion in comment #2 for version and release numbers. Even if upstream decides to make a 0.1 release we'll be in trouble. * The first 5 lines of the %build should probably go into %prep. Also could you use the "%setup -qcT" macro in %prep so that the package gets built in its own directory?
! ah. also it would be nice to have the source file downloaded with wget -N http://... so we have the correct timestamp.
Orcan, thanks for your review. My responses marked with + have been incorporated, -, not, and ? are pending your response (not yet incorporated). (In reply to comment #6) > * The license is MIT > https://fedoraproject.org/wiki/Licensing/MIT > old style. One of the oldest software licenses out there. + Thanks, missed that one. > ! Guidelines say: "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 must be included in %doc." in > http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#License_Text" > But this package does not even have a tarball. Is there any other examples of > Fedora packages that just cut the license text from the soure file? If not, I > would say it is best to obey the guideline. ? I'm unaware of any such practices (or cases where it might be applicable for that matter). I preferred to have the README out of the script in a %doc file, and figured when I did so, I could just as well generate a COPYING. It seems that I either should do it this way, or drop all generated %doc files entirely. Which option do you recommend? > ? Shall we rename the executable to just picturetile? That is the upstream > project name. - I agree that an extension of .pl is not necessary, but fspot looks for an executable picturetile.pl in $PATH, so unless we want a symlink from picturetile.pl to picturetile, I suggest to drop this comment. > ! It would be better to install the executable with "install -pm 755 ..." and > not use %attr(0755,root,root) to comply with general Fedora conventions.. + Agreed. > ? I am a bit reluctant about the provides: > Provides: picturetile.pl = 20050314 > Do we really need this? ? No, not really. I just added it in case someone decides to make a Requires in fspot. I can drop this without any adverse effects, should I? > ? I would go with the suggestion in comment #2 for version and release numbers. > Even if upstream decides to make a 0.1 release we'll be in trouble. + Ok. Although the version number is now fool proof, I doubt that it will be necessary. > * The first 5 lines of the %build should probably go into %prep. Also could you > use the "%setup -qcT" macro in %prep so that the package gets built in its own > directory? + Agreed. > ! ah. also it would be nice to have the source file downloaded with > wget -N http://... > so we have the correct timestamp. + Agreed. Though I unfortunately need some fussing around with timestamping since the file has to be converted to UTF-8. ? I'm not quite happy with the hardcoded URL in two places though. Isn't there a way to refer to Source0 directly (without rpm rewriting it to the local SOURCES directory)? Changes so far have been included in the following files: Spec URL: http://fedora.tenbrink-bekkers.nl/picturetile/picturetile.spec SRPM URL: http://fedora.tenbrink-bekkers.nl/picturetile/picturetile-0.1.20050314-3.fc11.src.rpm
(In reply to comment #8) > Orcan, thanks for your review. My responses marked with + have been > incorporated, -, not, and ? are pending your response (not yet incorporated). > You are welcome. I see that almost everything is resolved. Yet, there is a misunderstanding that was probably my fault. > (In reply to comment #6) > > But this package does not even have a tarball. Is there any other examples > > of Fedora packages that just cut the license text from the soure file? If > > not, I would say it is best to obey the guideline. > > ? I'm unaware of any such practices (or cases where it might be applicable for > that matter). I preferred to have the README out of the script in a %doc file, > and figured when I did so, I could just as well generate a COPYING. It seems > that I either should do it this way, or drop all generated %doc files > entirely. Which option do you recommend? > I am really not sure about this. The guideline might be interpreted either way. Could you ask this in the fedora-packaging list? > > ? Shall we rename the executable to just picturetile? That is the upstream > > project name. > > - I agree that an extension of .pl is not necessary, but fspot looks for an > executable picturetile.pl in $PATH, so unless we want a symlink from > picturetile.pl to picturetile, I suggest to drop this comment. > Your choice :) I am fine either way. > > > ? I am a bit reluctant about the provides: > > Provides: picturetile.pl = 20050314 > > Do we really need this? > > ? No, not really. I just added it in case someone decides to make a Requires > in fspot. I can drop this without any adverse effects, should I? > I would say, drop it. One RPM package providing two different versions of the same thing might confuse people or depsolvers. > > ? I would go with the suggestion in comment #2 for version and release > > numbers. Even if upstream decides to make a 0.1 release we'll be in trouble. > + Ok. Although the version number is now fool proof, I doubt that it will be necessary Oh I meant something like Version: 0 Release: 1.20050314 I prefer that we leave the Version to upstream vendors and use our conventions only in Release. But as you said, it is unlikely that this will cause a problem in the future, thus I leave this up to you. > > > ! ah. also it would be nice to have the source file downloaded with > > wget -N http://... > > so we have the correct timestamp. > > + Agreed. Though I unfortunately need some fussing around with timestamping > since the file has to be converted to UTF-8. > ? I'm not quite happy with the hardcoded URL in two places though. Isn't there > a way to refer to Source0 directly (without rpm rewriting it to the local > SOURCES directory)? > Here is the misunderstanding. No, you can't use wget in a spec file. koji has no outside access while building a package. Besides this may be considered as a security risk. What I meant was roughly: - Go to your SOURCES directory and use wget -N there, so that your original source0 file has the correct timestamp. - Then go to your SPECS directory and and use rpmbuild -ba (Don't use wget inside the SPEC file) - Now you can check your newly built rpm via rpm -qplv picturetile-<blah blah>.rpm and you will see that your /usr/bin/picturetile.pl file does have the correct timestamp. Sorry that I caused a confusion.
(In reply to comment #9) > (In reply to comment #8) > > > > > ? I am a bit reluctant about the provides: > > > Provides: picturetile.pl = 20050314 > > > Do we really need this? > > > > ? No, not really. I just added it in case someone decides to make a > > Requires in fspot. I can drop this without any adverse effects, should I? > > > > I would say, drop it. One RPM package providing two different versions of the > same thing might confuse people or depsolvers. > I realized that what I said is not true, since the package name is not "picturetile.pl". So having that Provides is harmless. I don't know if it brings us anything though. I leave it up to you.
(In reply to comment #9) > > (In reply to comment #6) > > > But this package does not even have a tarball. Is there any other examples > > > of Fedora packages that just cut the license text from the soure file? If > > > not, I would say it is best to obey the guideline. > > > > ? I'm unaware of any such practices (or cases where it might be applicable for > > that matter). I preferred to have the README out of the script in a %doc file, > > and figured when I did so, I could just as well generate a COPYING. It seems > > that I either should do it this way, or drop all generated %doc files > > entirely. Which option do you recommend? > > > > I am really not sure about this. The guideline might be interpreted either way. > Could you ask this in the fedora-packaging list? When writing my mail to fedora-packaging, I re-read the actual line several times. It seems I've over-interpreted it at the time of packaging: "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 must be included in %doc." The crux here is the *own file*. It does not include the text in it's own file COPYING. It is therefore incorrect to generate the file (because if the "If (and only if)"). Therefore, I'll strip out the generation of these files, as per your original suggestion.
This only applies to the COPYING filse though. You can keep the other one.
(In reply to comment #12) > This only applies to the COPYING filse though. You can keep the other one. As a gesture to the user, that would probably not be harmful. Agreed.
(In reply to comment #10) > (In reply to comment #9) > > (In reply to comment #8) > > > > > > > ? I am a bit reluctant about the provides: > > > > Provides: picturetile.pl = 20050314 > > > > Do we really need this? > > > > > > ? No, not really. I just added it in case someone decides to make a > > > Requires in fspot. I can drop this without any adverse effects, should I? > > > > > > > I would say, drop it. One RPM package providing two different versions of the > > same thing might confuse people or depsolvers. > > > > I realized that what I said is not true, since the package name is not > "picturetile.pl". So having that Provides is harmless. I don't know if it > brings us anything though. > > I leave it up to you. I'll leave it in, in order to make it easier for users to find this package. The typical use-case would be following: - user installs f-spot - user downloads additional extension PictureTile via fspot's Add-In manager - user runs Tools -> Create Photowall - user receives error message "picturetile.pl not found" - user tries to find "picturetile.pl", not "picturetile" Including the Provides makes it easier for rpmfind and the likes to serve this use-case.
Ok, I've incorporated all your comments. I'd like to go for a final review. New files can be found at: Spec URL: http://fedora.tenbrink-bekkers.nl/picturetile/picturetile.spec SRPM URL: http://fedora.tenbrink-bekkers.nl/picturetile/picturetile-0-4.20050314.fc11.src.rpm
It seems all good now. Thanks! ---------------------------------------------- This package (picturetile) is APPROVED by oget ----------------------------------------------
New Package CVS Request ======================= Package Name: picturetile Short Description: Tiles a bunch of images into one large "photo wall" Owners: edwintb Branches: F-12 InitialCC:
CVS done.
picturetile-0-4.20050314.fc12 has been submitted as an update for Fedora 12. http://admin.fedoraproject.org/updates/picturetile-0-4.20050314.fc12
picturetile-0-4.20050314.fc12 has been pushed to the Fedora 12 stable repository. If problems still persist, please make note of it in this bug report.