Bug 464781
Summary: | Review Request: flexdock - Java docking UI element. First package. | ||||||
---|---|---|---|---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | D Haley <mycae> | ||||
Component: | Package Review | Assignee: | Dominik 'Rathann' Mierzejewski <dominik> | ||||
Status: | CLOSED NEXTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> | ||||
Severity: | medium | Docs Contact: | |||||
Priority: | medium | ||||||
Version: | rawhide | CC: | chitlesh, edoutreleau, fedora-package-review, j, kwizart, mtasaka, notting, oget.fedora, sylvestre | ||||
Target Milestone: | --- | Flags: | dominik:
fedora-review+
kevin: fedora-cvs+ |
||||
Target Release: | --- | ||||||
Hardware: | All | ||||||
OS: | Linux | ||||||
Whiteboard: | |||||||
Fixed In Version: | 0.5.1-11.fc10 | Doc Type: | Bug Fix | ||||
Doc Text: | Story Points: | --- | |||||
Clone Of: | Environment: | ||||||
Last Closed: | 2009-03-23 15:50:21 UTC | Type: | --- | ||||
Regression: | --- | Mount Type: | --- | ||||
Documentation: | --- | CRM: | |||||
Verified Versions: | Category: | --- | |||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||
Cloudforms Team: | --- | Target Upstream Version: | |||||
Embargoed: | |||||||
Bug Depends On: | 469471 | ||||||
Bug Blocks: | 472639, 492203, 496433 | ||||||
Attachments: |
|
Description
D Haley
2008-09-30 13:53:06 UTC
I had to fiddle the %install section to get it to work, as the apache ant build.xml that is provided doesn't seem to have any facility for *installing* the package. Please read http://fedoraproject.org/wiki/Packaging/Guidelines and specially http://fedoraproject.org/wiki/Packaging/Java You have to use macro for the spec file (that what miss most to "start" review). Please also read http://fedoraproject.org/wiki/PackageMaintainers So you can get sponsored. For license problem, it is the one on top of each source file (if any) that matter. I have matio ready if you want to save time on scilab5 packaging...(i will submit it soon). (In reply to comment #0) > Spec URL: http://dhd.selfip.com/427e/flexdock.spec > SRPM URL: http://dhd.selfip.com/427e/flexdock-0.5.1-1.fc9.src.rpm > Description: flexdock java class files & JNI generated binary. Provides Swing > GUIs with a dock user interface element. Uploading as library is required for > (later) packaging of Scilab. Are you going to package Scilab, too, then? That's great. Let me know if I can help either with packaging or reviewing. I can also sponsor you, but please do a couple of reviews and point me to them. Yes, I am aiming to get a scilab package uploaded. I have a (private) basic scilab package all up and running (minus GUI), but I wanted to work out the approval/packaging process. The only thing its missing to get the scilab working in GUI mode is flexdock :) Nicolas: As of scilab 5, I believe matio is not directly required and scilab can (optionally) use its own code (or embedded code) to support these operations. (reference: http://wiki.scilab.org/Dependencies_of_Scilab_5.X). The java guidelines are a bit out of date, and keep talking about using the non sun java components (IcedTea, gcj); it's hard to tell what does and does not apply here. > You have to use macro for the spec file By macro I assume you mean that i should be installing the JNI .so files to %{_libdir}/%{name}? I can update the spec file accordingly. Otherwise if you are referring to the general format of the spec file, the original spec file was generated using vim, as per (my interpretation of) the packaging guidelines. Is something incorrect? Dominik: I read the guidelines a few times, for me the process is still a little fuzzy. I am following the procedure outlined here: http://fedoraproject.org/wiki/PackageMaintainers/Join, and am up to the part " Watch for Feedback" -- How do I fix the FE-NEEDSPONSOR -- It seems a little bit of a chicken and egg problem, I have to submit packages and get approval to get a sponsor, but can't do that until I have a sponsor... ? The guidelines say "When the package is APPROVED by the reviewer, you must separately obtain member sponsorship in order to check in and build your package." -- so shouldn't I be trying to get approval now??? Confused. Thanks for the help (In reply to comment #4) > Dominik: > I read the guidelines a few times, for me the process is still a little fuzzy. > I am following the procedure outlined here: > http://fedoraproject.org/wiki/PackageMaintainers/Join, and am up to the part " > Watch for Feedback" -- How do I fix the FE-NEEDSPONSOR -- It seems a little bit > of a chicken and egg problem, I have to submit packages and get approval to get > a sponsor, but can't do that until I have a sponsor... ? You can submit packages and do non-authoritative reviews before getting sponsored. In fact, that's the best way to convince a someone (me, for example) to sponsor you. After that person approves you for membership in the "packager" group in the Fedora Account System, you will be able to import and build any approved package. > The guidelines say "When the package is APPROVED by the reviewer, you must > separately obtain member sponsorship in order to check in and build your > package." -- so shouldn't I be trying to get approval now??? Confused. You should find someone to sponsor you. Usually, sponsorship happens at the same time as your first package is approved. As I said, I can do that after I'm satisfied that you have learned to follow our packaging guidelines. I'll take the review of this package now. Hello, Sorry about that, but the spec file I uploaded was completely bogus: don't bother officially reviewing the package at this stage :(. I have modified the spec file to create a jar file and install that, rather than installing all the .class files everywhere. Unfortunately my examination of the package has shown more problems than just .class files. Problem: Flexdock depends on several other libraries: *Java media framework http://java.sun.com/javase/technologies/desktop/media/jmf/ ./lib/jmf/lib/jmf.jar ./lib/jmf/lib/mediaplayer.jar ./lib/jmf/lib/multiplayer.jar ./lib/jmf/lib/customizer.jar -- *"Looks" project https://looks.dev.java.net/ ./lib/looks-2.1.1.jar *skinlf "Skin look and Feel" project https://skinlf.dev.java.net/ ./lib/skinlf.jar Each of which don't appear to have fedora packages at this stage. Unless I am allowed to use the .jar files supplied with flexdock, flexdock cannot build. I have uploaded the new .spec file in case anyone has any comments. Note that the ant-jmf seems to cause a build failure, and thus cannot be used: compile: [javac] Compiling 229 source files to /home/makerpm/rpmbuild/BUILD/flexdock-0.5.1/build/bin [javac] Note: Some input files use or override a deprecated API. [javac] Note: Recompile with -Xlint:deprecation for details. [javac] Compiling 29 source files to /home/makerpm/rpmbuild/BUILD/flexdock-0.5.1/build/bin-demo [javac] /home/makerpm/rpmbuild/BUILD/flexdock-0.5.1/src/java/demo/org/flexdock/demos/raw/jmf/MediaPanel.java:21: package javax.media does not exist [javac] import javax.media.ControllerEvent; [javac] ^ Am I supposed to now create packages for each of the missing deps? If someone can tell me what I am supposed to do now, that would be great! Thanks. Well, https://fedoraproject.org/wiki/Packaging/Java#Pre-built_JAR_files_.2F_Other_bundled_software Removing all pre-compiled .jar files (.class files etc) at %prep is _mandatory_ on Fedora to make it sure that all files are correctly built from FOSS file. If this package needs some external jar files you must package them in advance (example: bug 428798 , especially the discussion from bug 428798 comment 11 ) Looks like this might be dead in the water. Unless this is out of date, the JMF is available under the "Sun Community Source Licensing", which is not allowed in fedora per http://fedoraproject.org/wiki/Licensing. Might have to raise this as a bug at scilab. Sorry about the dup, forgot the JMF FAQ link: http://java.sun.com/javase/technologies/desktop/media/jmf/reference/faqs/index.html#jmfsource Well, while I cannot find out yet where to download JMF source file itself, as far as I read the link in your comment 9 jmf cannot be included in Fedora. This has been reported as a bug at scilab, as this package is intended to support scilab-5.0.1 http://bugzilla.scilab.org/show_bug.cgi?id=3696 Looks like you got an answer from the Debian maintainer of flexdock along with some patches to fix the problem. Please go ahead and incorporate them into your package and post a new spec+source rpm set for review. I have uploaded the new spec & src rpm. Now pending upon deps. You are supposed to increase the revision of the rpm (and document any changes in the %changelog section) and post the complete urls for spec and srpm in the bugzilla comment. mycae, if you modified your spec/srpm, would you change the release number and post the URLs of your new spec/srpm? Spec URL: http://dhd.selfip.com/427e/flexdock-4.spec SRPM URL: http://dhd.selfip.com/427e/flexdock-0.5.1-4.fc9.src.rpm - Update package revision number - Fix Top level dir # # Copyright (c) 2008 Daniel Haley # # This file and all modifications and additions to the pristine # package are under the same license as the package itself. # # please send bugfixes or comments to mycae # Hmm isn't this a blocker for review ? There should be no other copyright/license on any fedora spec file except fedora's. #001: Missing BuildRequires: ant-commons-logging #002: Rpmlint issues; chitlesh(~)[0]$rpmlint /home/chitlesh/rpmbuild/RPMS/i386/flexdock-0.5.1-4.fc9.i386.rpm flexdock.i386: W: no-documentation flexdock.i386: W: non-standard-group Development/Java flexdock.i386: W: incoherent-version-in-changelog 0.1-4 ['0.5.1-4.fc9', '0.5.1-4'] flexdock.i386: W: no-soname /usr/lib/libRubberBand.so flexdock.i386: E: shlib-with-non-pic-code /usr/lib/libRubberBand.so 1 packages and 0 specfiles checked; 1 errors, 4 warnings. chitlesh(~)[0]$rpmlint /home/chitlesh/rpmbuild/RPMS/i386/flexdock-debuginfo-0.5.1-4.fc9.i386.rpm flexdock-debuginfo.i386: E: empty-debuginfo-package #001: missing ant-commons-logging as Build Requires #002: add -verbose to ant Spec URL: http://dhd.selfip.com/427e/flexdock-6.spec SRPM URL: http://dhd.selfip.com/427e/flexdock-0.5.1-6.fc9.src.rpm >Hmm isn't this a blocker for review ? There should be no other >copyright/license on any fedora spec file except fedora's. Gone. I thought this was part of the package licence's requirements -- but its not. >#001: Missing BuildRequires: ant-commons-logging Fixed. >#002: Rpmlint issues; rpmlint for SRPM,spec and RPM are empty. >#001: missing ant-commons-logging as Build Requires Fixed. >#002: add -verbose to ant Added, but this produces an extremely long output -- significantly greater than the scrollback on my terminal. Not good for building unless you (1) log or (2) redirect or both. Known issues: *I am not actually removing the skinlf jar, as the code to do this is commented out. I discussed this with akurtakov previously & will revisit this soon. * Thu Dec 04 2008 <mycae(a!t)yahoo.com> 0.5.1-6 - Use ant to build jar, rather than straight from bash. - Fix build patch to correct native .so building - Thanks goes to akurtakov for assitance. - Fixed dos->unix line ending for doc files - Added dos2unix buildrequires - Fix arch flag - Added libX11 dep for native so - Added symlink for so For the record, it is acceptable for specfiles to be explicitly licensed as long as that license is accepted for software in Fedora: http://fedoraproject.org/wiki/Licensing#License_of_Fedora_SPEC_Files Spec URL: http://dhd.selfip.com/427e/flexdock-7.spec SRPM URL: http://dhd.selfip.com/427e/flexdock-0.5.1-7.fc9.src.rpm *Sat Dec 20 2008 <mycae(a!t)yahoo.com> 0.5.1-7 - Re-enable system skinlf link & jar check. spec file, SRPM and RPM give empty rpmlint output This was updated as skinlf has now been fixed & flexdock now builds correctly. Some comments before full review: Why do you have: BuildArch: i386 BuildArch: x86-64 BuildArch: ppc in the spec? BuildArch: can only be specified once, so merge these three. Otherwise it fails with: $ rpmbuild -bb flexdock.spec error: No compatible architectures found for build x86-64 is not a valid arch designation, by the way. You should use x86_64. I think you should use ExcludeArch: ppc64 to disable the unsupported arch instead and open a bug for that (blocking FE-ExcludeArch-ppc64 tracker bug). Fails to build in mock on rawhide/i386: [...] ++ build-classpath jgoodies-looks skinlf /usr/bin/build-classpath: error: Could not find jgoodies-looks Java extension for this JVM /usr/bin/build-classpath: error: Could not find skinlf Java extension for this JVM /usr/bin/build-classpath: error: Some specified jars were not found [...] You seem to have forgotten to add these as BuildRequires as well. Once the above are fixed, it builds in mock, but rpmlint output is not clean: $ rpmlint /var/lib/mock//fedora-rawhide-i386/result flexdock-debuginfo.i386: E: empty-debuginfo-package flexdock.src:157: E: files-attr-not-set flexdock.src:158: E: files-attr-not-set flexdock.src:159: E: files-attr-not-set 3 packages and 0 specfiles checked; 4 errors, 0 warnings. You need to fix that. According to Java Packaging Guidelines, you're installing the files in the wrong location. Please fix that. Also, the specfile has lots of trailing whitespace and inconsistent usage of spaces and tabs for indentation. Please fix that, too. Removing FE-NEEDSPONSOR, as you are already sponsored. Hopefully this package is near completion. I must be close to winning a most revised package award. SPEC URL: http://dhd.selfip.com/427e/flexdock-8.spec SRPM URL: http://dhd.selfip.com/427e/flexdock-0.5.1-8.fc10.src.rpm Now that deps are satisified, I have provided a scratch build. Koji scratch: http://koji.fedoraproject.org/koji/taskinfo?taskID=1068723flexdock-0.5.1-8.fc10.src.rpm I swear that the rpmlint is emty. really! [makerpm@box SPECS]$ rpmlint ../SRPMS/flexdock-0.5.1-8.fc10.src.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. [makerpm@box SPECS]$ rpmlint ../RPMS/i386/flexdock-0.5.1-8.fc10.i386.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. [makerpm@box SPECS]$ rpmlint flexdock.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings. >$ rpmbuild -bb flexdock.spec >error: No compatible architectures found for build Now I'm not sure why I have not left this out. I see no reason that PPC64 is special, at least not considering the changes to the sdk.home. >You seem to have forgotten to add these as BuildRequires as well. Added >$ rpmlint /var/lib/mock//fedora-rawhide-i386/result >flexdock-debuginfo.i386: E: empty-debuginfo-package >flexdock.src:157: E: files-attr-not-set[makerpm@spiderbox SPECS]$ rpmlint -i ../RPMS/i386/flexdock-0.5.1-8.fc10.i386.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. [makerpm@spiderbox SPECS]$ rpmlint -i ../SRPMS/flexdock-0.5.1-8.fc10.src.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. [makerpm@spiderbox SPECS]$ rpmlint -i flexdock.spec 0 packages and 1 specfiles checked; 0 errors, 0 warnings. >flexdock.src:158: E: files-attr-not-set >flexdock.src:159: E: files-attr-not-set >3 packages and 0 specfiles checked; 4 errors, 0 warnings. Defattr set, strip command modified to only unneeded, rather than all. >According to Java Packaging Guidelines, you're installing the files in the >wrong location. Please fix that. Re-Reading: http://fedoraproject.org/wiki/Packaging/Java#Packaging_JAR_files_that_use_JNI I have moved them into their own subfolder /%{_libdir}/%{name}/. The origin of this particular rationale is somewhat unclear to me. Having examined a long thread from March 2007 (http://osdir.com/ml/linux.redhat.fedora.java/2007-03/msg00044.html), consensus was at best unclear. Nevertheless, they are moved. >Also, the specfile has lots of trailing whitespace and inconsistent usage of >spaces and tabs for indentation. Please fix that, too. Trailing whitespace is now gone -- I dont see any indentation issues (grep), as such this has not been fixed. Also rpmlint warns mixed tab/space indenting -- am I not understanding something? $ grep -E '^\ ' flexdock.spec $ There was a trailing endline, and I got rid of it. Some more comments: Requires: libX11 >= 1.1.4 Why do you need a specific libX11 version? Please add a comment with an explanation. Also, please sort both Requires: and BuildRequires: alphabetically for better readability. %description FlexDock is a Java docking framework for use in cross-platform Swing applications Why is the description indented? Also, it's missing the end period. #Modify the jni dir that is hardcoded in the patch cp -pf %{PATCH0} %{PATCH0}.tmp sed -i 's!%%{_libdir}/%%{name}/!%{_libdir}/%{name}/!' %{PATCH0} Why don't you just modify the patch? if [ `uname -m` == "x86_64" ] || [ `uname -m` == "ppc64" ] ; then JDK_DIR=`echo %{_jdkdir} | sed 's!/$!!'`.`uname -m` I suggest using a case statement here. It'll be easy to add other arches later. For example sparc64. #OK # Apache commons Logging component # http://commons.apache.org/logging/ # ./lib/commons-logging-1.1.jar rm -f ./lib/commons-logging-1.1.jar ln -s %{_javadir}/commons-logging.jar ./lib/commons-logging-1.1.jar #remove jmf, as it is only used in a demo, #which is unused after patching rm -rf ./lib/jmf/ #" Looks" project # https://looks.dev.java.net/ rm -f ./lib/looks-2.1.1.jar ln -s %{_javadir}/jgoodies-looks.jar ./lib/looks-2.2.1.jar #skinlf "Skin look and Feel" project # https://skinlf.dev.java.net/ rm -f ./lib/skinlf.jar ln -s %{_javadir}/skinlf.jar ./lib/skinlf.jar Manually making symlinks is fragile, especially for versioned JARs. It'll break when their version changes. I think you should use build-jar-repository for that: https://fedoraproject.org/wiki/Packaging/Java#build-jar-repository dos2unix -d2u $i; What does -d2u option do? It's not documented in --help or in the manpage. flexdock has funny arch flags, such as "libRubberBand-linux-x86.so" on i386 SOFILE=`find ./ -name libRubberBand*so` Couldn't you just patch the build system not to put arch in lib filename? strip --strip-unneeded $SOFILE Calling strip from the specfile is not allowed. rpm post-build scripts do that while building the debuginfo package. %post -p /sbin/ldconfig %postun -p /sbin/ldconfig These are not needed, as the library is not in ld.so search path (and not directly linkable). *Tue Jan 20 2009 <mycae(a!t)yahoo.com> 0.5.1-8 - Set defattr - Fix arch (again) - Change install dir from %%libdir to %%libdir/%%name - Update patch0 to match changed so dir + make dynamic *Sat Dec 20 2008 <mycae(a!t)yahoo.com> 0.5.1-7 - Re-enable system skinlf link & jar check. Changelog entries must have a space between * and date. (In reply to comment #26) [...] > strip --strip-unneeded $SOFILE > > Calling strip from the specfile is not allowed. rpm post-build scripts do that > while building the debuginfo package. Although in this case they seem to fail. If I remove that strip call, I get: $ rpmlint /var/lib/mock//fedora-rawhide-i386/result flexdock-debuginfo.i386: E: empty-debuginfo-package flexdock.i386: W: unstripped-binary-or-object /usr/lib/flexdock/libRubberBand-0.so (In reply to comment #27) > (In reply to comment #26) > [...] > > strip --strip-unneeded $SOFILE > > > > Calling strip from the specfile is not allowed. rpm post-build scripts do that > > while building the debuginfo package. > > Although in this case they seem to fail. If I remove that strip call, I get: > $ rpmlint /var/lib/mock//fedora-rawhide-i386/result > flexdock-debuginfo.i386: E: empty-debuginfo-package > flexdock.i386: W: unstripped-binary-or-object > /usr/lib/flexdock/libRubberBand-0.so Actually it's simple. Just don't chmod the .so to 644. It has to be executable for rpmbuild debuginfo scripts to find it. rpmlint is silent afterwards. Some more nitpicks: You don't need mkdir -p %{buildroot}/usr/share There's still trailing whitespace in the following lines: Name: flexdock # This patch is fedora specific -- System.loadLibrary fix to help locate JNI components #Licence is MIT on their website, Apache in their LICENSE.txt patch3: flexdock-skinlfTitlebarui-path.patch patch4: flexdock-skinlfPainter-path.patch BuildRequires: java-1.6.0-openjdk-devel Requires: libX11 >= 1.1.4 JDK_DIR=`echo %{_jdkdir} | sed 's!/$!!'`.`uname -m` echo "Relying on spec file buildpath: $JDK_DIR " >> tmpLog echo "sdk.home=$JDK_DIR" > workingcopy.properties # https://skinlf.dev.java.net/ ant -Dbuild.sysclasspath=first build.with.native jar SPEC URL: http://dhd.selfip.com/427e/flexdock-9.spec SRPM URL: http://dhd.selfip.com/427e/flexdock-0.5.1-9.fc10.src.rpm Scratch: http://koji.fedoraproject.org/koji/taskinfo?taskID=1073687 $ rpmlint flexdock.spec ../RPMS/i386/flexdock-0.5.1-9.fc10.i386.rpm ../SRPMS/flexdock-0.5.1-9.fc10.src.rpm 2 packages and 1 specfiles checked; 0 errors, 0 warnings. >There's still trailing whitespace in the following lines I re-ran the whitespace removal through vim. What is so bad about trailing whitespace anyway - provided it isn't multi-kilobyte? >mkdir -p %{buildroot}/usr/share Gone. >Why don't you just modify the patch? Because of lib64 vs lib. >-d2u It forces dos to unix conversion mode. I now just use sed instead, its neater and is one less require. >Couldn't you just patch the build system not to put arch in lib filename? The way they do this, from memory is a bit odd. Patching would require more work than simply locating the SOFILE -- locating the SOFILE is easier and the project is not changing that much, and the SOFILE name wont change a lot, so it is maintainable. Either way the end result is the same -- we locate the SOFILE. Using find is marginally slower, but I am not writing my spec files for speed. >strip --strip-unneeded $SOFILE Calling strip is not allowed because you need the debugging symbols, however calling strip-unneeded does not intefere with the debugging symbols. However to make it neater I have changed this (see next item) > Actually it's simple. Just don't chmod the .so to 644. It has to be executable > for rpmbuild debuginfo scripts to find it. rpmlint is silent afterwards. Done. >Manually making symlinks is fragile, especially for versioned JARs. Versioned jars were not being symlinked, although named ones were. The version number is to satisfy the flexdock buildsys. I have switched it to using build-jar-repository, but I feel that the new method is less straightforward, and more likely to break. I personally think the new method is more obscure. >These are not needed, as the library is not in ld.so search path (and not directly linkable). Dropped post/postun >I suggest using a case statement here. It'll be easy to add other arches later. >For example sparc64. Done. > Changelog entries must have a space between * and date. Done. >Why do you need a specific libX11 version? Gone. I cant seem to track down any reason for this. Created attachment 331996 [details] Suggested specfile fixes Full review: rpmlint is clean $ rpmlint . 3 packages and 0 specfiles checked; 0 errors, 0 warnings. * MUST: The package must meet the Packaging Guidelines. %define _jdkdir %{_jvmdir}/java-1.6.0-openjdk-1.6.0.0 I suggest using simply %{_jvmdir}/java-1.6.0 instead, which will allow you to drop the JDK_DIR=`echo %{_jdkdir} | sed 's!/$!!'`.`uname -m` hack later (see attached patch). Is java-1.6 (not older and not newer) strictly required? Patch0: flexdock-jni-patch.patch + File file = new File("%{_libdir}/%{name}/");^ That '/' at the end is not necessary. Also the patch file name has a redundant 'patch' in it, same for others. BuildRequires: jpackage-utils is listed twice. #Override the build file's default hard-coded paths if [ x"$JAVACMD" != x"" ] ; then echo "using RPM jnidir" >> tmpLog echo sdk.home=%{_jnidir}-`$JAVACMD -version` > workingcopy.properties else if [ x"$JAVA_HOME" != x"" ] ; then echo "Using JAVA_HOME env. var. :" $JAVA_HOME >> tmpLog echo "sdk.home=$JAVA_HOME" > workingcopy.properties else JDK_DIR=%{_jdkdir} echo "Relying on spec file buildpath: $JDK_DIR " >> tmpLog echo "sdk.home=$JDK_DIR" > workingcopy.properties fi fi Why is the above necessary instead of: echo "sdk.home=%{_jvmdir}/java-1.6.0" > workingcopy.properties You could lose the %define _jdkdir at the beginning of the specfile then, too. Also see the attached patch for more cosmetic fixes. * MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines. Most files have no licencing information while others are licenced under MIT licence. Given the presence of LICENSE.txt, this is OK, but please ask upstream to include licencing information at the top of each source file * MUST: The License field in the package spec file must match the actual license. #Licence is MIT on their website, Apache in their LICENSE.txt License: MIT and ASL 2.0 Wrong. LICENSE.txt is actually word-for-word MIT: https://fedoraproject.org/wiki/Licensing/MIT#Modern_Style_with_sublicense * MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. Reviewers should use md5sum for this task. If no upstream URL can be specified for this package, please see the Source URL Guidelines for how to deal with this. 88fd43d7d8db92e9480200c316e55056 flexdock-0.5.1-src.zip * SHOULD: The reviewer should test that the package builds in mock. * SHOULD: The package should compile and build into binary rpms on all supported architectures. Doesn't build on F-9/x86_64 and F-9/i386 (java bug?). Apologies for delay in the response, haven't had a chance to look at this in the past few days. SPEC URL: http://dhd.selfip.com/427e/flexdock-10.spec SRPM URL: http://dhd.selfip.com/427e/flexdock-0.5.1-10.fc10.src.rpm Scratch: F9:http://koji.fedoraproject.org/koji/taskinfo?taskID=1139730 F10:http://koji.fedoraproject.org/koji/taskinfo?taskID=1139731 $ rpmlint ../SRPMS/flexdock-0.5.1-10.fc10.src.rpm flexdock.spec ../RPMS/i386/flexdock-0.5.1-10.fc10.i386.rpm 2 packages and 1 specfiles checked; 0 errors, 0 warnings. >#Licence is MIT on their website, Apache in their LICENSE.txt >License: MIT and ASL 2.0 >Wrong. LICENSE.txt is actually word-for-word MIT: >https://fedoraproject.org/wiki/Licensing/MIT#Modern_Style_with_sublicense Fixed. >Doesn't build on F-9/x86_64 and F-9/i386 (java bug?). Between changes from my earlier f-9 srpms to now, a new buildrequires was needed. Added: BuildRequires: ant-apache-regexp Hence the build for F9 is now fixed. >That '/' at the end is not necessary. Also the patch file name has a redundant >'patch' in it, same for others. I don't see the problem with having such things there, but to aid review process I have removed these. >Why is the above necessary instead of: >echo "sdk.home=%{_jvmdir}/java-1.6.0" > workingcopy.properties Changed. This was based upon a jpackage script. >BuildRequires: jpackage-utils >is listed twice. >Also see the attached patch for more cosmetic fixes. Applied, with thanks. >Is java-1.6 (not older and not newer) strictly required? No, this was part of the hack. Changed to java-devel, java and %{_jvmdir}/java Very nice, however... the %changelog entry you've just added contains an empty line: - Yes, I know I'm being awfully picky. ;) Of course you can fix it after importing, because this package is now APPROVED. Great work and thanks for bearing with me. Looks like I was a bit too hasty. Please remove the jmf library from the original source archive. We can't distribute that, it's non-free. While you're at it, you could also remove the other binaries, since you have to repackage the source archive anyway. SPEC URL: http://dhd.selfip.com/427e/flexdock-11.spec SRPM URL: http://dhd.selfip.com/427e/flexdock-0.5.1-11.fc10.src.rpm Koji scratch: http://koji.fedoraproject.org/koji/taskinfo?taskID=1154545 rpmlint: $ rpmlint ../SRPMS/flexdock-0.5.1-11.fc10.src.rpm ../RPMS/i386/flexdock-0.5.1-11.fc10.i386.rpm flexdock.spec 2 packages and 1 specfiles checked; 0 errors, 0 warnings. >Please remove the jmf library from the original source archive Done. > you could also remove the other binaries Done. The JMF has the nice side effect of reducing size of package. ;) >Very nice, however... the %changelog entry you've just added contains an empty >line: Removed. >Thanks for bearing with me. Whatever makes for good packages is a good thing. One last small nitpick: BuildRequires: jpackage-utils is listed twice. Looks like it slipped in after I asked you to fix it in comment #30. Please fix it again upon import. This package is APPROVED. New Package CVS Request ======================= Package Name: flexdock Short Description: Docking framework for Java Swing GUI apps Owners: mycae Branches: F-9 F-10 InitialCC: cvs done. flexdock-0.5.1-11.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/flexdock-0.5.1-11.fc10 flexdock-0.5.1-11.fc9 has been submitted as an update for Fedora 9. http://admin.fedoraproject.org/updates/flexdock-0.5.1-11.fc9 flexdock-0.5.1-11.fc9 has been pushed to the Fedora 9 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing-newkey update flexdock'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F9/FEDORA-2009-2321 flexdock-0.5.1-11.fc10 has been pushed to the Fedora 10 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update flexdock'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-2342 Could this package be moved to stable and the review bug closed as NEXTRELEASE? flexdock-0.5.1-11.fc9 has been pushed to the Fedora 9 stable repository. If problems still persist, please make note of it in this bug report. flexdock-0.5.1-11.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report. Could you please add an unversioned symlink to the versioned jar file inside %{_libdir} ? Something like ln -s %{name}-%{version}.jar %{buildroot}%{_libdir}/%{name}/%{name}.jar Most java packages carry these symlinks and they provide convenience on depending packages (Have a look at your /usr/share/java/ for instance). Thanks. >Could you please add an unversioned symlink to the versioned jar file inside >%{_libdir} ? Not for a JNI component as I understand. https://fedoraproject.org/wiki/Packaging/Java#Packaging_JAR_files_that_use_JNI I don't get that implication from those guidelines. I'm packaging frinika (bug 492203). For this one, I had to put your jar file in the classpath. But what will happen if there's a minor update in flexdock? Let's say upstream makes a minor bugfix release 0.5.1.1. In this case your jar file will be renamed, and I have to change my classpath and rebuild my package. But I shouldn't have to update my package for such a case. For instance, take a look at our libreadline-java package. OK, but could you explain where I am misunderstanding? The bit that I thought was important is this:
>Second, placing the JAR file in %{_javadir} causes the build-classpath script to >always load it, even when running on a runtime environment of the wrong arch, >meaning that the System.loadLibrary line would fail.
I will have a look at libreadline-java in the next few days
Yes, but that has nothing to do with what I am saying. I say: create a symlink /usr/lib64/flexdock/flexdock.jar that points to /usr/lib64/flexdock/flexdock-0.5.1.jar This has nothing to do with %{_javadir}, which is /usr/share/java. So the build-classpath script will still not see it. Hi Orcan, Yes, just to clarify, I pushed that in as an update. Sorry I misread your original post as asking for a link in /usr/share/java/ No problem, thanks for taking care of this :) |