[Bug 228960] Review Request: java-1.5.0-gcj - JPackage compatibility layer for GCJ

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]