Spec URL: http://www.guthrie.info/RPMS/f9/simplyhtml.spec SRPM URL: http://www.guthrie.info/RPMS/f9/simplyhtml-0.12.3-2.fc9.src.rpm Description: SimplyHTML is an application and a java component for rich text processing. It stores documents as HTML files in combination with Cascading Style Sheets (CSS). SimplyHTML is not intended to be used as an editor for web pages.
This package has a dependency on javahelp2 which currently exists in the devel tree. Thus, this package would be targeted only for rawhide and F10. (I'm not certain how does a case like this get handled.)
(Using Jason Tibbitts' review template from http://fedoraproject.org/wiki/User:Tibbs/Review_Template) [-] source files match upstream: I followed the instructions in the spec file and ended up with something with a different md5sum than the file in the SRPM. The readme.txt and gpl.txt files do match though You could just use the upstream .zip file directly, though. Just change your %setup line to %setup -q -c %{name}-%{version} and it'll create the directory when it needs to. [+] package meets naming and versioning guidelines. [+] specfile is properly named, is cleanly written and uses macros consistently. [+] dist tag is present. [+] build root is correct. [+] license field matches the actual license. [+] license is open source-compatible. [+] license text included in package. [-] latest version is being packaged. It looks like upstream has released 0.12.5 [+] BuildRequires are proper. [+] compiler flags are appropriate. [+] %clean is present. [-] package builds in mock. error: %patch without corresponding "Patch:" tag You should have "%patch0", not "%patch", on line 62 For the remainder of this review I made this change [+] package installs properly. [+] debuginfo package looks complete. [-] rpmlint is silent. One warning to deal with: simplyhtml.src: W: mixed-use-of-spaces-and-tabs (spaces: line 3, tab: line 20) The specfile mixes use of spaces and tabs for indentation, which is a cosmetic annoyance. Use either spaces or tabs for indentation, not both. Other warnings are about Group tags or confusion with the gcj noarch-ness [+] final provides and requires are sane: Provides: simplyhtml-0.12.3.jar.so simplyhtml = 0:0.12.3-2.fc10 simplyhtml(x86-32) = 0:0.12.3-2.fc10 Requires: /bin/sh gnu-regexp java java-gcj-compat >= 1.0.31 javahelp2 jpackage-utils libc.so.6 libc.so.6(GLIBC_2.1.3) libdl.so.2 libgcc_s.so.1 libgcc_s.so.1(GCC_3.0) libgcj_bc.so.1 libm.so.6 libpthread.so.0 librt.so.1 libz.so.1 rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 rtld(GNU_HASH) [+] no shared libraries are added to the regular linker search paths. [+] owns the directories it creates. [+] doesn't own any directories it shouldn't. [+] no duplicates in %files. [+] file permissions are appropriate. [+] gcj scriplets are correct [+] code, not content. [+] documentation is small, so no -docs subpackage is necessary. [+] %docs are not necessary for the proper functioning of the package. [+] no headers. [+] no pkgconfig files. [+] no libtool .la droppings. [-] Package doesn't run I recommend creating a small shell script to run the program with the correct CLASSPATH -- if you just try to run the jar file, it doesn't find the gnu-regexp classes. Something like this: #!/bin/sh exec java -cp `build-classpath gnu-regexp javahelp2 simplyhtml` \ com.lightdev.app.shtm.App [-] Other source files are included: Please check the status and the necessity of using the files in src/com/sun and de/calcom as they appear to come from other projects.
(In reply to comment #2) > (Using Jason Tibbitts' review template from > http://fedoraproject.org/wiki/User:Tibbs/Review_Template) > > [-] source files match upstream: > I followed the instructions in the spec file and ended up with > something with a different md5sum than the file in the SRPM. The > readme.txt and gpl.txt files do match though > > You could just use the upstream .zip file directly, though. Just change > your %setup line to > %setup -q -c %{name}-%{version} > and it'll create the directory when it needs to. That would probably make my life noticeably easier. > [-] latest version is being packaged. > It looks like upstream has released 0.12.5 That's not too surprising since I first posted this review request 4.5 months ago. Let me download the new source and rebuild. > [-] Package doesn't run > I recommend creating a small shell script to run the program with the > correct CLASSPATH -- if you just try to run the jar file, it doesn't > find the gnu-regexp classes. > > Something like this: > > #!/bin/sh > > exec java -cp `build-classpath gnu-regexp javahelp2 simplyhtml` \ > com.lightdev.app.shtm.App When and where would I want to do this? During compile-time perhaps? (There are other blockers that I haven't addressed in this message. I was just picking a few blockers to deal with at first.) I will post a new SRPM and spec soon.
(In reply to comment #3) > (In reply to comment #2) > > [-] Package doesn't run > > I recommend creating a small shell script to run the program with the > > correct CLASSPATH -- if you just try to run the jar file, it doesn't > > find the gnu-regexp classes. > > > > Something like this: > > > > #!/bin/sh > > > > exec java -cp `build-classpath gnu-regexp javahelp2 simplyhtml` \ > > com.lightdev.app.shtm.App > > When and where would I want to do this? During compile-time perhaps? Never mind this comment. I was totally mis-parsing what you were trying to say. I figured it out. ;-)
(In reply to comment #2) I have posted a new spec file and SRPM. Here are the URLs: Spec: http://www.guthrie.info/RPMS/f10/simplyhtml.spec SRPM: http://www.guthrie.info/RPMS/f10/simplyhtml-0.12.5-2.fc10.src.rpm > [-] source files match upstream: > I followed the instructions in the spec file and ended up with > something with a different md5sum than the file in the SRPM. The > readme.txt and gpl.txt files do match though > > You could just use the upstream .zip file directly, though. Just change > your %setup line to > %setup -q -c %{name}-%{version} > and it'll create the directory when it needs to. For the 0.12.5 version, the upstream is now offering the source in .tar.gz format. I am now using that unmodified. > [-] latest version is being packaged. > It looks like upstream has released 0.12.5 This is fixed. 0.12.5 is now packaged. > [-] package builds in mock. > error: %patch without corresponding "Patch:" tag > You should have "%patch0", not "%patch", on line 62 > For the remainder of this review I made this change This is fixed. > [-] rpmlint is silent. > One warning to deal with: > simplyhtml.src: W: mixed-use-of-spaces-and-tabs (spaces: line 3, tab: line > 20) > The specfile mixes use of spaces and tabs for indentation, which is a > cosmetic > annoyance. Use either spaces or tabs for indentation, not both. This is fixed. > [-] Package doesn't run > I recommend creating a small shell script to run the program with the > correct CLASSPATH -- if you just try to run the jar file, it doesn't > find the gnu-regexp classes. > > Something like this: > > #!/bin/sh > > exec java -cp `build-classpath gnu-regexp javahelp2 simplyhtml` \ > com.lightdev.app.shtm.App I did this almost verbatim except that I added simplyhtml-help to the build-classpath command. > [-] Other source files are included: > Please check the status and the necessity of using the files in > src/com/sun and de/calcom as they appear to come from other projects. The short story is that from what I can tell, these files actually are imported into the files for this project. I will talk more about this later when it is not 2AM and I am needing sleep. ;-)
(In reply to comment #5) > (In reply to comment #2) > > [-] Other source files are included: > > Please check the status and the necessity of using the files in > > src/com/sun and de/calcom as they appear to come from other projects. > > The short story is that from what I can tell, these files actually are imported > into the files for this project. I will talk more about this later when it is > not 2AM and I am needing sleep. ;-) This could end up being a bit of a mess. Just looking at the file com/sun/demo/ExampleFileFilter.java, the class defined by this file gets imported into three different files under com.lightdev. Moreover, these files use methods that are defined in ExampleFileFilter.java. It would be difficult to extricate this class from the com.lightdev tree without making semi-substantial changes to the code. On further inspection, one also sees that the ExampleFileFilter class extends the FileFilter class. The documentation for that class can be found here: http://java.sun.com/j2se/1.4.2/docs/api/javax/swing/filechooser/FileFilter.html According to that page, the FileFilter class is in fact an abstract class, so it is expected that a subclass will be written in order to implement this class. It appears that this particular subclass, ExampleFileFilter, was written to, well, be an example implementation. However, I am guessing that the example code did everything that the simplyhtml upstream wanted, and so they just used it directly, rather than writing their own implementation. Fortunately, ExampleFileFilter.java has a 3-clause BSD license with no advertising clause. Similar comments apply to ElementTreePanel.java. All of the de.calcom code has been placed under GPL just like the rest of the com.lightdev code. IT turns out that the lightdev.com and calcom.de domains are owned by the same person, one Ulrich Hilger. Moreover, looking at the calcom.de web site, it would appear that calcom.de was a previous name for lightdev.com. Based on this, it would seem safe to assume that the de.calcom code is part of the "same" project as the com.lightdev code. Is there anything that we would need to worry about with the com.sun.demo code having a BSD license vs. the rest of the code having a GPL license?
I asked on fedora-devel-java-list about the com.sun.* classes, and unfortunately, it looks like since the license of these files includes * You acknowledge that this software is not designed, licensed or intended * for use in the design, construction, operation or maintenance of any * nuclear facility. they can't be used in Fedora. Here's the mailing list thread: https://www.redhat.com/archives/fedora-devel-java-list/2009-February/msg00008.html For the ExampleFileFilter, it doesn't look like it would be too difficult to patch the code to use the javax.swing.filechooser.FileNameExtensionFilter that was introduced in JDK 1.6 and that provides similar functionality. For the ElementTreePanel, I'm not sure if there's an alternative implementation, but I haven't looked too hard. In the worst case, I guess you could patch out the use of the ElementTreePanel entirely, and then there would be some functionality missing from simplyhtml.
(In reply to comment #7) > I asked on fedora-devel-java-list about the com.sun.* classes, and > unfortunately, it looks like since the license of these files includes > * You acknowledge that this software is not designed, licensed or intended > * for use in the design, construction, operation or maintenance of any > * nuclear facility. > they can't be used in Fedora. Here's the mailing list thread: > > https://www.redhat.com/archives/fedora-devel-java-list/2009-February/msg00008.html > > For the ExampleFileFilter, it doesn't look like it would be too difficult to > patch the code to use the javax.swing.filechooser.FileNameExtensionFilter that > was introduced in JDK 1.6 and that provides similar functionality. > > For the ElementTreePanel, I'm not sure if there's an alternative > implementation, but I haven't looked too hard. In the worst case, I guess you > could patch out the use of the ElementTreePanel entirely, and then there would > be some functionality missing from simplyhtml. I'm sitting here chuckling to myself a little bit because I saw that clause, and I actually thought to myself, "Yeah, I can understand why someone might want to keep Java code out of a nuclear facility. <sarcasm>Mmmmm... Java code and nuclear power plants. Two tastes that go great together.</sarcasm>" I can also understand now why this might be a bad thing from Fedora's point of view. Thanks for the pointer on FileNameExtensionFilter. I'll see what I can do with that for now. I'll also have to keep that "no nukes" clause in mind as well. ;-)
Good news -- looks like there's a newer version of ElementTreePanel included in the java-1.6.0-openjdk-demo package inside /usr/lib/jvm/java-1.6.0-openjdk-1.6.0.0/demo/jfc/Notepad/src.zip. According to the fedora-devel-java mailing list (same thread as before), you should just be able to delete the included version and copy this one in instead. The license will have to become something like "GPLv2+ and BSD" (http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#Multiple_Licensing_Scenarios).
(In reply to comment #9) > Good news -- looks like there's a newer version of ElementTreePanel included in > the java-1.6.0-openjdk-demo package inside > /usr/lib/jvm/java-1.6.0-openjdk-1.6.0.0/demo/jfc/Notepad/src.zip. According to > the fedora-devel-java mailing list (same thread as before), you should just be > able to delete the included version and copy this one in instead. The license > will have to become something like "GPLv2+ and BSD" > (http://fedoraproject.org/wiki/Packaging/LicensingGuidelines#Multiple_Licensing_Scenarios). That is SOOOO cool. I was very seriously considering deleting the class and removing the code from the package. Thank you so much for finding this replacement. In any event, I have successfully replaced ExampleFileFilter with FileNameExtensionFilter. The result can be seen here: http://www.guthrie.info/RPMS/f10/simplyhtml-0.12.5-3.fc10.src.rpm. The new spec file has the same URL as given above.
What would be the proper/preferred way to include this newer ElementTreePanel.java file? Would I want to just extract the file myself and include it in the SRPM as another source file, or would I want to specify a java-1.6.0-openjdk-demo as a BuildRequirement, and then extract the java file from that package during the %prep phase? (The java file would not be included with the SRPM in the latter option, of course.)
Sorry, $DAYJOB exploded on me for a couple of weeks there. :) It's more elegant to BuildRequire the demo package and move the file where you need it (+ change package, whatever), but I don't think it would be wrong to just include the file itself as a separate Source: file in the SRPM. Whichever you prefer, I'd say.
(In reply to comment #12) > Sorry, $DAYJOB exploded on me for a couple of weeks there. :) No problem. I suppose that it's good that $DAYJOB != "". ;-) > It's more elegant to BuildRequire the demo package and move the file where you > need it (+ change package, whatever), but I don't think it would be wrong to > just include the file itself as a separate Source: file in the SRPM. Whichever > you prefer, I'd say. That's what I was thinking. I didn't know if there was any downside though. Here is the new SRPM URL with the new BuildRequire: http://www.guthrie.info/RPMS/f10/simplyhtml-0.12.5-4.fc10.src.rpm The Spec file URL is still the same, but updated.
Looks good. Just one tiny rpmlint warning from the SRPM: simplyhtml.src: W: strange-permission simplyhtml.sh 0755 A file that you listed to include in your package has strange permissions. Usually, a file should have 0644 permissions. Since you have the %attr line in the %files section, I don't think that the source file needs any special permissions itself. This is APPROVED (but please change the permissions on that one source file before importing, just for neatness. :) )
Thank you very much. That last permission change can be seen at http://www.guthrie.info/RPMS/f10/simplyhtml-0.12.5-5.fc10.src.rpm for reference. Onto the CVS procedure now... Again, thank you for all of your help.
New Package CVS Request ======================= Package Name: simplyhtml Short Description: Application and a java component for rich text processing Owners: guthrie Branches: F-9 F-10 InitialCC:
cvs done.
It now builds successfully in CVS. Closing.
Package Change Request ====================== Package Name: simplyhtml New Branches: F-11 Owners: guthrie
Never mind. I was trying to check out the new branch incorrectly. I figured it out now.