Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report. Summary: Review Request: java-1.5.0-gcj - JPackage compatibility layer for GCJ https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=228960 ------- Additional Comments From fitzsim@xxxxxxxxxx 2007-03-11 04:49 EST ------- (In reply to comment #3) > Initial review (more to come): > > Comments: > > . I'd like to see a separate sinjdoc SRPM https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=231732 I added a bootstrap macro for use before sinjdoc is built. > . around line 306 there are some "../../../../.." paths - can this be > done in a less fragile manner? The result is actually more robust, but I agree that counting ../'s is ugly. > If not, can we get a comment that specifies why/what we're doing? I added a new macro to make the absolute-to-relative conversion automatic and documented it at the top of the spec file. > . I'm fine with the commented-out plugin sections but perhaps note why > we've done this I uncommented those sections, wrapped them with an enable_plugin macro and added a comment at the top of the spec file. > . why have the %define gccver 4.1.2 commented out? is this due to the > fact that it wasn't buildable until that version hit rawhide? Yes that was the reason. I've uncommented it. > > MUST: > * package is named appropriately > * it is legal for Fedora to distribute this > . I guess my only concern here would be the use of the word java in > the name but I guess that's okay? > ? license field matches the actual license. > . it would be nice if there were an actual webpage ... even just a > simple page listing the license and with links to source drops OK, probably a good idea -- I'll look into this, but let's not let it hold up the review. > * license is open source-compatible. > * specfile name matches %{name} > * source and patches verified > . it would be nice if the j-g-c patches could be rolled upstream; Good point, done. > or commented if that's not possible > * summary and description okay > * correct buildroot > * %{?dist} used properly > * license text included in package and marked with %doc > ? packages meets FHS (http://www.pathname.com/fhs/) Yes, it does, except maybe tools.jar, which is architecture-independent data and should therefore be installed under /usr/share. That said, I'm currently working upstream to replace tools.jar with a symbolic link to libgcj-tools.jar, which will bring the package into full compliance. > * rpmlint on <this package>.srpm gives no output > E: java-1.5.0-gcj hardcoded-library-path in %{_prefix}/lib > . justified in spec comments and in review request > * changelog fine > * Packager tag not used > * Vendor tag not used > * Distribution tag not used > * License and not Copyright used > * Summary tag does not end in a period > * if possible, replace PreReq with Requires(pre) and/or Requires(post) > * specfile is legible > * package successfully compiles and builds on at least x86 > * BuildRequires are proper > * summary is a short and concise description of the package > * description expands upon summary > * make sure lines are <= 80 characters > . just the first python macro line and a %files entry or two - fine > * specfile written in American English > * no -doc sub-package necessary (In reply to comment #4) > A few more comments: > > . can you explain the need for the triggerins? Is it just in case a > newer set of gcc RPMs is installed and we need to have > explicitly-versioned symlinks? Yes. java-1.5.0-gcj maintains versionless symlinks to files in gcc's versioned directories. If gcc's version changes underneath java-1.5.0-gcj, then the symlinks need to be recreated, since the old ones would point to non-existent directories. > . in %files javadoc, you have a comment that says that > %{_javadocdir}/java will conflict with other packages owning it - is > there no alternative entry for that directory? Should there be? The original comment wasn't precise. I wrote a new one to clarify: # A JPackage that "provides" this directory will, in its %post script, # remove the existing directory and install a new symbolic link to its # versioned directory. For Fedora we want clear file ownership so we # make java-1.5.0-gcj-javadoc own this file. Installing the # corresponding JPackage over java-1.5.0-gcj-javadoc will work but # will invalidate this file. %doc %{_javadocdir}/java > > * no static libs > * no rpath > * config files should marked with %config(noreplace) > * not a GUI app > * sub-packages fine (-devel, -src, etc.) > * macros used appropriately and consistently > - ie. %{buildroot} and %{optflags} vs. $RPM_BUILD_ROOT and $RPM_OPT_FLAGS > * %makeinstall not used > * no locale data > * Requires(pre,post) fine > * package not relocatable > * package contains code > * package owns all directories and files > * no %files duplicates > * file permissions okay; %defattrs present > * %clean present > * %doc files do not affect runtime > * not a webapp > ? verify the final provides and requires of the binary RPMs > > Once we fix the GIJ_VERSION at the top of the specfile, I think the requires > will be okay (ie. the >= 4.0.0 will become the correct version). Do you think > we should explicitly require the version and not have a >= for it? No, the >= is necessary. Otherwise we'd have to release java-1.5.0-gcj in lock-step with gcc releases. > > $ rpm -qp --requires > /home/andrew/rpmbuild/RPMS/i386/java-1.5.0-gcj-1.5.0.0-1.i386.rpm > /bin/bash > /bin/sh > /bin/sh > /bin/sh > /usr/bin/gcj-dbtool > /usr/bin/gcj-dbtool > /usr/bin/gij > /usr/bin/gij > /usr/bin/rebuild-gcj-db > /usr/bin/rebuild-gcj-db > /usr/bin/rebuild-security-providers > /usr/bin/rebuild-security-providers > /usr/sbin/alternatives > /usr/sbin/alternatives > config(java-1.5.0-gcj) = 1.5.0.0-1 > jpackage-utils >= 1.7.3 > libgcj >= 4.0.0 > mx4j >= 3.0.1 > rpmlib(CompressedFileNames) <= 3.0.4-1 > rpmlib(PayloadFilesHavePrefix) <= 4.0-1 > rpmlib(VersionedDependencies) <= 3.0.3-1 > > $ rpm -qp --requires > /home/andrew/rpmbuild/RPMS/i386/java-1.5.0-gcj-devel-1.5.0.0-1.i386.rpm > /bin/sh > /bin/sh > /bin/sh > /bin/sh > /usr/bin/env > /usr/bin/gcj > /usr/sbin/alternatives > /usr/sbin/alternatives > eclipse-ecj >= 3.2.1 > gcc-java >= 4.0.0 > java-1.5.0-gcj = 1.5.0.0-1 > java_cup >= 0.10 > python >= 2.5 > python(abi) = 2.5 > rpmlib(CompressedFileNames) <= 3.0.4-1 > rpmlib(PayloadFilesHavePrefix) <= 4.0-1 > rpmlib(VersionedDependencies) <= 3.0.3-1 > > $ rpm -qp --requires > /home/andrew/rpmbuild/RPMS/i386/java-1.5.0-gcj-src-1.5.0.0-1.i386.rpm > java-1.5.0-gcj = 1.5.0.0-1 > libgcj-src >= 4.0.0 > /usr/bin/gij > rpmlib(VersionedDependencies) <= 3.0.3-1 > /bin/sh > /bin/sh > /bin/sh > rpmlib(PayloadFilesHavePrefix) <= 4.0-1 > rpmlib(CompressedFileNames) <= 3.0.4-1 > > $ rpm -qp --requires > /home/andrew/rpmbuild/RPMS/i386/java-1.5.0-gcj-javadoc-1.5.0.0-1.i386.rpm > java-1.5.0-gcj = 1.5.0.0-1 > rpmlib(CompressedFileNames) <= 3.0.4-1 > rpmlib(PayloadFilesHavePrefix) <= 4.0-1 > rpmlib(VersionedDependencies) <= 3.0.3-1 > > * run rpmlint on the binary RPMs > > $ rpmlint /home/andrew/rpmbuild/RPMS/i386/java-1.5.0-gcj-1.5.0.0-1.i386.rpm > E: java-1.5.0-gcj only-non-binary-in-usr-lib > W: java-1.5.0-gcj dangling-relative-symlink > /usr/share/java/gcj-endorsed/mx4j-remote.jar ../mx4j/mx4j-remote.jar > W: java-1.5.0-gcj dangling-relative-symlink > /usr/lib/jvm/java-1.5.0-gcj-1.5.0.0/jre/bin/rmiregistry > ../../../../../bin/grmiregistry > W: java-1.5.0-gcj dangling-relative-symlink > /usr/lib/jvm/java-1.5.0-gcj-1.5.0.0/jre/lib/security/java.security > ../../../../../security/classpath.security > W: java-1.5.0-gcj dangling-relative-symlink > /usr/lib/jvm-exports/java-1.5.0-gcj-1.5.0.0/jaas-1.5.0.0.jar > ../../jvm/java-1.5.0-gcj-1.5.0.0/jre/lib/jaas.jar > W: java-1.5.0-gcj dangling-relative-symlink > /usr/lib/jvm-exports/java-1.5.0-gcj-1.5.0.0/jdbc-stdext-1.5.0.0.jar > ../../jvm/java-1.5.0-gcj-1.5.0.0/jre/lib/jdbc-stdext.jar > W: java-1.5.0-gcj dangling-relative-symlink > /usr/lib/jvm-exports/java-1.5.0-gcj-1.5.0.0/jsse-1.5.0.0.jar > ../../jvm/java-1.5.0-gcj-1.5.0.0/jre/lib/jsse.jar > W: java-1.5.0-gcj dangling-relative-symlink > /usr/lib/jvm/java-1.5.0-gcj-1.5.0.0/jre/bin/keytool ../../../../../bin/gkeytool > W: java-1.5.0-gcj dangling-relative-symlink > /usr/lib/jvm-exports/java-1.5.0-gcj-1.5.0.0/jndi-1.5.0.0.jar > ../../jvm/java-1.5.0-gcj-1.5.0.0/jre/lib/jndi.jar > W: java-1.5.0-gcj dangling-relative-symlink > /usr/share/java/gcj-endorsed/mx4j.jar ../mx4j/mx4j.jar > W: java-1.5.0-gcj dangerous-command-in-%post ln > W: java-1.5.0-gcj dangerous-command-in-%trigger ln > > $ rpmlint /home/andrew/rpmbuild/RPMS/i386/java-1.5.0-gcj-devel-1.5.0.0-1.i386.rpm > E: java-1.5.0-gcj-devel only-non-binary-in-usr-lib > W: java-1.5.0-gcj-devel no-documentation > W: java-1.5.0-gcj-devel dangling-relative-symlink > /usr/lib/jvm/java-1.5.0-gcj-1.5.0.0/bin/jarsigner ../../../../bin/gjarsigner > W: java-1.5.0-gcj-devel dangling-relative-symlink > /usr/lib/jvm/java-1.5.0-gcj-1.5.0.0/bin/java ../../../../bin/gij > W: java-1.5.0-gcj-devel dangling-relative-symlink > /usr/lib/jvm/java-1.5.0-gcj-1.5.0.0/bin/jar ../../../../bin/fastjar > W: java-1.5.0-gcj-devel dangling-relative-symlink > /usr/lib/jvm/java-1.5.0-gcj-1.5.0.0/bin/rmic ../../../../bin/grmic > W: java-1.5.0-gcj-devel dangling-relative-symlink > /usr/lib/jvm/java-1.5.0-gcj-1.5.0.0/bin/rmiregistry ../../../../bin/grmiregistry > W: java-1.5.0-gcj-devel dangling-relative-symlink > /usr/lib/jvm/java-1.5.0-gcj-1.5.0.0/bin/keytool ../../../../bin/gkeytool > W: java-1.5.0-gcj-devel dangling-relative-symlink > /usr/lib/jvm/java-1.5.0-gcj-1.5.0.0/bin/javac ../../../../bin/ecj > W: java-1.5.0-gcj-devel dangling-relative-symlink > /usr/lib/jvm/java-1.5.0-gcj-1.5.0.0/bin/javah ../../../../bin/gjavah > W: java-1.5.0-gcj-devel dangling-relative-symlink > /usr/lib/jvm/java-1.5.0-gcj-1.5.0.0/bin/appletviewer ../../../../bin/gappletviewer > W: java-1.5.0-gcj-devel dangling-relative-symlink > /usr/lib/jvm-exports/java-1.5.0-gcj java-1.5.0-gcj-1.5.0.0 > W: java-1.5.0-gcj-devel dangerous-command-in-%post ln > W: java-1.5.0-gcj-devel dangerous-command-in-%trigger ln > > $ rpmlint /home/andrew/rpmbuild/RPMS/i386/java-1.5.0-gcj-javadoc-1.5.0.0-1.i386.rpm > $ rpmlint /home/andrew/rpmbuild/RPMS/i386/java-1.5.0-gcj-src-1.5.0.0-1.i386.rpm > W: java-1.5.0-gcj-src no-documentation > W: java-1.5.0-gcj-src dangerous-command-in-%post ln > W: java-1.5.0-gcj-src dangerous-command-in-%postun rm > W: java-1.5.0-gcj-src dangerous-command-in-%trigger ln > > > SHOULD: > * package should include license text in the package and mark it with %doc > * package should build on i386 > ? package should build in mock > I haven't tried I'll run a mock build and post the results. The new spec and SRPM files are posted at the same URLs. -- Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are the QA contact for the bug, or are watching the QA contact. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review