[Bug 1187713] netty-tcnative
https://bugzilla.redhat.com/show_bug.cgi?id=1187713 Zbigniew Jędrzejewski-Szmek changed: What|Removed |Added Flags|fedora-review? |fedora-review+ --- Comment #20 from Zbigniew Jędrzejewski-Szmek --- (In reply to jiri vanek from comment #19) > Last thing we disagree is the arch/noarch lib/path/lib64 convention. > > I'm still keeping my approach, beacuse I somehow feel that what you wont me > to do is wrong: > > - from fedora guidelines the multilib jni packages are not supported > - my main package contains both jar and nativelib => not noarch > > if I split natvie and java files to two packages and made jar "multilib" > (search in both lib and lib64) then yes, your case you shown with dnf will > work. However, > - the multilib is not supposed to be supported > - there is real danger that jar run in 64 jvm will load 32b library or vice > versa (afaik it is why jni is not supported multilib) > - the jar without so and vice versa dont have sense. OK. This feels to be a suboptimal situation to me, but I don't see an easy solution. I was wrong in thinking that the packages for the two architectures can be installed simultaneously when the patch to use .load() is modified to not include an architecture-specific path. It turns out that the jar contains headers which include a timestamp, so the resulting jar files are different anyway. A work-around could be added, but it's not trivial. I believe all issues have been fixed or determined to be unfixable. Package is APPROVED. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1187713] netty-tcnative
https://bugzilla.redhat.com/show_bug.cgi?id=1187713 --- Comment #19 from jiri vanek --- (In reply to Zbigniew Jędrzejewski-Szmek from comment #18) > (In reply to jiri vanek from comment #17) > > (In reply to Zbigniew Jędrzejewski-Szmek from comment #16) > > > The patch to load libraries is wrong. It makes the jar file architecture > > > dependent, > > yes, intentionally > > > > > but it should not be. Just try loading from /usr/lib64, and then from > > > /usr/lib. > > > This will work on both 64 and 32 bit archs. > > > > Well - this patch is fedora only, and fedora do not support multilib jni > > packages > > (http://fedoraproject.org/wiki/Packaging: > > Java#Packaging_JAR_files_that_use_JNI - uttermost end - ". Java packages > > using JNI do not support multiarch installation." > > > > Although I see your point, I would rather stay with my approach - to fail > > immediately if wrong arch is found. > > > > If you disagree, and insists, I will follow your opinion. > I do disagree. It generates an unnecessary conflict (which is detected > only during transaction check, which is nasty for users): > > $ sudo dnf install results/netty-tcnative-1.1.30-0.fc22.x86_64.rpm > ... > Error: Transaction check error: > file /usr/lib/java/netty-tcnative/netty-tcnative.jar from install of > netty-tcnative-1.1.30-0.fc22.x86_64 conflicts with file from package > netty-tcnative-1.1.30-0.fc22.i686 > file /usr/share/maven-metadata/netty-tcnative.xml from install of > netty-tcnative-1.1.30-0.fc22.x86_64 conflicts with file from package > netty-tcnative-1.1.30-0.fc22.i686 > ...snip... > > > This seems unnecessary. > > This seems to be autoadded > > -javadoc should be noarch! I think it might go away when the > package is noarch. javadoc made noarch. spec: https://jvanek.fedorapeople.org/elasticsearch/v4/netty-tcnative/netty-tcnative.spec srpm: https://jvanek.fedorapeople.org/elasticsearch/v4/netty-tcnative/netty-tcnative-1.1.30-0.fc21.src.rpm Last thing we disagree is the arch/noarch lib/path/lib64 convention. I'm still keeping my approach, beacuse I somehow feel that what you wont me to do is wrong: - from fedora guidelines the multilib jni packages are not supported - my main package contains both jar and nativelib => not noarch if I split natvie and java files to two packages and made jar "multilib" (search in both lib and lib64) then yes, your case you shown with dnf will work. However, - the multilib is not supposed to be supported - there is real danger that jar run in 64 jvm will load 32b library or vice versa (afaik it is why jni is not supported multilib) - the jar without so and vice versa dont have sense. Thank you for patience! -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1187713] netty-tcnative
https://bugzilla.redhat.com/show_bug.cgi?id=1187713
--- Comment #18 from Zbigniew Jędrzejewski-Szmek ---
(In reply to jiri vanek from comment #17)
> (In reply to Zbigniew Jędrzejewski-Szmek from comment #16)
> > The patch to load libraries is wrong. It makes the jar file architecture
> > dependent,
> yes, intentionally
>
> > but it should not be. Just try loading from /usr/lib64, and then from
> > /usr/lib.
> > This will work on both 64 and 32 bit archs.
>
> Well - this patch is fedora only, and fedora do not support multilib jni
> packages
> (http://fedoraproject.org/wiki/Packaging:
> Java#Packaging_JAR_files_that_use_JNI - uttermost end - ". Java packages
> using JNI do not support multiarch installation."
>
> Although I see your point, I would rather stay with my approach - to fail
> immediately if wrong arch is found.
>
> If you disagree, and insists, I will follow your opinion.
I do disagree. It generates an unnecessary conflict (which is detected
only during transaction check, which is nasty for users):
$ sudo dnf install results/netty-tcnative-1.1.30-0.fc22.x86_64.rpm
...
Error: Transaction check error:
file /usr/lib/java/netty-tcnative/netty-tcnative.jar from install of
netty-tcnative-1.1.30-0.fc22.x86_64 conflicts with file from package
netty-tcnative-1.1.30-0.fc22.i686
file /usr/share/maven-metadata/netty-tcnative.xml from install of
netty-tcnative-1.1.30-0.fc22.x86_64 conflicts with file from package
netty-tcnative-1.1.30-0.fc22.i686
> > [!]: License file installed when any subpackage combination is installed.
> > No license is installed with -javadocs.
>
> Eh... No license file does exists in whole project.
Ah, OK.
> >
> > [!]: Package requires other packages for directories it uses.
> > Note: No known owner of /usr/share/maven-poms/netty-tcnative,
> > /usr/lib64
> > /netty-tcnative, /usr/lib/java/netty-tcnative
> > [!]: Package must own all directories that it creates.
> > Note: Directories without known owners: /usr/share/maven-poms/netty-
> > tcnative, /usr/lib64/netty-tcnative, /usr/lib/java/netty-tcnative
> > Those should be added.
>
> fixed (sorry, I thought mvn_install is handling those three)
> >
> ...snip...
> > file
> > from upstream, the packager SHOULD query upstream to include it.
> > [!]: Final provides and requires are sane (see attachments).
> > See comments below
> >
> > [x]: Fully versioned dependency in subpackages if applicable.
> > Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in netty-
> > tcnative-javadoc
> > Not needed.
> >
> > [?]: Package functions as described.
>
> The testclass runs fine on both x86_64 and i386
I wasn't trying to say that the package does not work, but only that I didn't
test it :)
> > [x]: Latest version is packaged.
> > [x]: Package does not include license text files separate from upstream.
> > [x]: Patches link to upstream bugs/comments/lists or are otherwise
> > justified.
> > [-]: Description and summary sections in the package spec file contains
> > translations for supported Non-English languages, if available.
> > [?]: Package should compile and build into binary rpms on all supported
> > architectures.
> I have not tested arms...
>
> > I don't think it would actually build, because of the binary conflict.
>
> What do you mean?/Wy do you think so?
I thought that it would detect the conflict between files. Apparently it does
not, the check is only done for noarch packages. So the build is OK.
> > Requires
> >
> > netty-tcnative (rpmlib, GLIBC filtered):
> > apr
> > java-headless
> > jpackage-utils
> > libapr-1.so.0()(64bit)
> > libc.so.6()(64bit)
> > libcrypto.so.10()(64bit)
> > libcrypto.so.10(OPENSSL_1.0.1)(64bit)
> > libcrypto.so.10(OPENSSL_1.0.1_EC)(64bit)
> > libcrypto.so.10(libcrypto.so.10)(64bit)
> > libdl.so.2()(64bit)
> > libpthread.so.0()(64bit)
> > libssl.so.10()(64bit)
> > libssl.so.10(libssl.so.10)(64bit)
> > openssl
> > This also seems unecessary. IIUC, only ssl libs are required, and that
> > dependency is provided automatically.
> Ok, I have removed (how had you found it?)
It links to the library, but does not seems to call any binaries directly.
> Requires: apr
> Requires: openssl
> But added
> Requires: java-headless
> Requires: jpackage-utils
> (http://fedoraproject.org/wiki/Packaging:Java#BuildRequires_and_Requires ?)
I think they are added automatically because of the BR:maven-local,
but adding them explicitly does not hurt.
> >
> > rtld(GNU_HASH)
> >
> > netty-tcnative-javadoc (rpmlib, GLIBC filtered):
> > jpackage-utils
> > This seems unnecessary.
> This seems to be autoadded
-javadoc should be noarch! I think it might go away when the
package is noarch.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package
[Bug 1187713] netty-tcnative
https://bugzilla.redhat.com/show_bug.cgi?id=1187713
--- Comment #17 from jiri vanek ---
(In reply to Zbigniew Jędrzejewski-Szmek from comment #16)
> The patch to load libraries is wrong. It makes the jar file architecture
> dependent,
yes, intentionally
> but it should not be. Just try loading from /usr/lib64, and then from
> /usr/lib.
> This will work on both 64 and 32 bit archs.
Well - this patch is fedora only, and fedora do not support multilib jni
packages
(http://fedoraproject.org/wiki/Packaging:Java#Packaging_JAR_files_that_use_JNI
- uttermost end - ". Java packages using JNI do not support multiarch
installation."
Although I see your point, I would rather stay with my approach - to fail
immediately if wrong arch is found.
If you disagree, and insists, I will follow your opinion.
>
> = MUST items =
>
> [!]: License file installed when any subpackage combination is installed.
> No license is installed with -javadocs.
Eh... No license file does exists in whole project.
>
> [!]: Package requires other packages for directories it uses.
> Note: No known owner of /usr/share/maven-poms/netty-tcnative, /usr/lib64
> /netty-tcnative, /usr/lib/java/netty-tcnative
> [!]: Package must own all directories that it creates.
> Note: Directories without known owners: /usr/share/maven-poms/netty-
> tcnative, /usr/lib64/netty-tcnative, /usr/lib/java/netty-tcnative
> Those should be added.
fixed (sorry, I thought mvn_install is handling those three)
>
...snip...
> file
> from upstream, the packager SHOULD query upstream to include it.
> [!]: Final provides and requires are sane (see attachments).
> See comments below
>
> [x]: Fully versioned dependency in subpackages if applicable.
> Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in netty-
> tcnative-javadoc
> Not needed.
>
> [?]: Package functions as described.
The testclass runs fine on both x86_64 and i386
> [x]: Latest version is packaged.
> [x]: Package does not include license text files separate from upstream.
> [x]: Patches link to upstream bugs/comments/lists or are otherwise justified.
> [-]: Description and summary sections in the package spec file contains
> translations for supported Non-English languages, if available.
> [?]: Package should compile and build into binary rpms on all supported
> architectures.
I have not tested arms...
> I don't think it would actually build, because of the binary conflict.
What do you mean?/Wy do you think so?
>
...
>
>
> Rpmlint
> ---
> Checking: netty-tcnative-1.1.30-0.fc22.x86_64.rpm
> netty-tcnative-javadoc-1.1.30-0.fc22.x86_64.rpm
> netty-tcnative-1.1.30-0.fc22.src.rpm
> netty-tcnative.x86_64: W: name-repeated-in-summary C Netty-tcnative
> netty-tcnative.x86_64: W: spelling-error %description -l en_US mavenization
> -> magnetization, humanization, maximization
> netty-tcnative.x86_64: W: incoherent-version-in-changelog 1.1.30.Fork2.0
> ['1.1.30-0.fc22', '1.1.30-0']
> ???
>
humph. No Named tags allowed? Oook. followed.
> netty-tcnative.x86_64: W: no-documentation
> netty-tcnative.src: W: name-repeated-in-summary C Netty-tcnative
> rpmlint is right here. The summary is meaningless.
>
fixed
> netty-tcnative.src: W: spelling-error %description -l en_US mavenization ->
> magnetization, humanization, maximization
> netty-tcnative.src: W: strange-permission netty-tcnative-1.1.30.Fork2.tar.gz
> 0640L
> netty-tcnative.src:68: W: macro-in-comment %{_jnidir}
> netty-tcnative.src:68: W: macro-in-comment %{name}
> netty-tcnative.src:68: W: macro-in-comment %{name}
> Please fix those.
done
>
> netty-tcnative.src:62: W: mixed-use-of-spaces-and-tabs (spaces: line 4, tab:
> line 62)
> And this.
also fixed.
>
> netty-tcnative.src: W: patch-not-applied Patch1: fixLibNames.patch.in
> 3 packages and 0 specfiles checked; 0 errors, 12 warnings.
> OK.
>
>
> Rpmlint (installed packages)
>
> Cannot parse rpmlint output:
>
>
> Requires
>
> netty-tcnative (rpmlib, GLIBC filtered):
> apr
> java-headless
> jpackage-utils
> libapr-1.so.0()(64bit)
> libc.so.6()(64bit)
> libcrypto.so.10()(64bit)
> libcrypto.so.10(OPENSSL_1.0.1)(64bit)
> libcrypto.so.10(OPENSSL_1.0.1_EC)(64bit)
> libcrypto.so.10(libcrypto.so.10)(64bit)
> libdl.so.2()(64bit)
> libpthread.so.0()(64bit)
> libssl.so.10()(64bit)
> libssl.so.10(libssl.so.10)(64bit)
> openssl
> This also seems unecessary. IIUC, only ssl libs are required, and that
> dependency is provided automatically.
Ok, I have removed (how had you found it?)
Requires: apr
Requires: openssl
But added
Requires: java-headless
Requires: jpackage-utils
(http://fedoraproject.org/wiki/Packaging:Java#BuildRequires_and_Requires ?)
>
> rtld(GNU_HASH)
>
> netty-tcnative-javadoc (rpmlib, GLIBC filtered):
> jpackage-utils
> This seems unnecessary.
This seems to be autoadded
>
>
> Provides
> ---
[Bug 1187713] netty-tcnative
https://bugzilla.redhat.com/show_bug.cgi?id=1187713
--- Comment #16 from Zbigniew Jędrzejewski-Szmek ---
The patch to load libraries is wrong. It makes the jar file architecture
dependent,
but it should not be. Just try loading from /usr/lib64, and then from /usr/lib.
This will work on both 64 and 32 bit archs.
= MUST items =
C/C++:
[x]: Package does not contain kernel modules.
[x]: Package contains no static executables.
[x]: Development (unversioned) .so files in -devel subpackage, if present.
Note: Unversioned so-files in private %_libdir subdirectory (see
attachment). Verify they are not in ld path.
[x]: Package does not contain any libtool archives (.la)
[x]: Rpath absent or only used for internal libs.
Generic:
[x]: Package is licensed with an open-source compatible license and meets
other legal requirements as defined in the legal section of Packaging
Guidelines.
[x]: 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 is included in %doc.
[x]: License field in the package spec file matches the actual license.
Note: Checking patched sources after %prep for licenses. Licenses found:
"Apache (v2.0)", "*No copyright* Apache (v2.0)". Detailed output of
licensecheck in /var/tmp/1187713-netty-tcnative/licensecheck.txt
[!]: License file installed when any subpackage combination is installed.
No license is installed with -javadocs.
[!]: Package requires other packages for directories it uses.
Note: No known owner of /usr/share/maven-poms/netty-tcnative, /usr/lib64
/netty-tcnative, /usr/lib/java/netty-tcnative
[!]: Package must own all directories that it creates.
Note: Directories without known owners: /usr/share/maven-poms/netty-
tcnative, /usr/lib64/netty-tcnative, /usr/lib/java/netty-tcnative
Those should be added.
[x]: %build honors applicable compiler flags or justifies otherwise.
[x]: Package contains no bundled libraries without FPC exception.
[x]: Changelog in prescribed format.
[x]: Sources contain only permissible code or content.
[-]: Package contains desktop file if it is a GUI application.
[-]: Development files must be in a -devel package
[x]: Package uses nothing in %doc for runtime.
[x]: Package consistently uses macros (instead of hard-coded directory names).
[x]: Package is named according to the Package Naming Guidelines.
[x]: Package does not generate any conflict.
[x]: Package obeys FHS, except libexecdir and /usr/target.
[-]: If the package is a rename of another package, proper Obsoletes and
Provides are present.
[x]: Requires correct, justified where necessary.
[x]: Spec file is legible and written in American English.
[-]: Package contains systemd file(s) if in need.
[x]: Useful -debuginfo package or justification otherwise.
[x]: Package is not known to require an ExcludeArch tag.
[x]: Large documentation must go in a -doc subpackage. Large could be size
(~1MB) or number of files.
Note: Test run failed
[x]: Packages must not store files under /srv, /opt or /usr/local
Note: Test run failed
[x]: Package complies to the Packaging Guidelines
[x]: Package successfully compiles and builds into binary rpms on at least one
supported primary architecture.
[x]: Package installs properly.
[x]: Rpmlint is run on all rpms the build produces.
Note: There are rpmlint messages (see attachment).
[x]: Package does not own files or directories owned by other packages.
[x]: All build dependencies are listed in BuildRequires, except for any that
are listed in the exceptions section of Packaging Guidelines.
[x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT
[x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the
beginning of %install.
[x]: Macros in Summary, %description expandable at SRPM build time.
[x]: Package does not contain duplicates in %files.
[x]: Permissions on files are set properly.
[x]: Package use %makeinstall only when make install' ' DESTDIR=... doesn't
work.
[x]: Package is named using only allowed ASCII characters.
[x]: Package do not use a name that already exist
[x]: Package is not relocatable.
[x]: Sources used to build the package match the upstream source, as provided
in the spec URL.
[x]: Spec file name must match the spec package %{name}, in the format
%{name}.spec.
[x]: File names are valid UTF-8.
Java:
[x]: Bundled jar/class files should be removed before build
Note: Test run failed
[x]: Packages have proper BuildRequires/Requires on jpackage-utils
Note: Maven packages do not need to (Build)Require jpackage-utils. It is
pulled in by maven-local
[x]: Javadoc documentation files are generated and included in -javadoc
subpackage
[x]: Javadoc subpackages should not have Requires: jpackage-utils
[x]: Javadocs are placed in %{_javadocdir}/%{name} (no -%{version} symlink)
Maven:
[x]: If package cont
[Bug 1187713] netty-tcnative
https://bugzilla.redhat.com/show_bug.cgi?id=1187713 --- Comment #15 from Zbigniew Jędrzejewski-Szmek --- (In reply to jiri vanek from comment #14) > btw, why are .la dangerous? They aren't dangerous, but they are useless on Linux (linking works just fine without them, so they just obfuscate), but they shouldn't be removed in stable updates (iiuc, other .la files created based on this library will refer to the .la file). The only reason they are often installed is that libtool cannot be told not to install them. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1187713] netty-tcnative
https://bugzilla.redhat.com/show_bug.cgi?id=1187713 --- Comment #14 from jiri vanek --- btw, why are .la dangerous? -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1187713] netty-tcnative
https://bugzilla.redhat.com/show_bug.cgi?id=1187713 --- Comment #13 from jiri vanek --- Directory updated: spec: https://jvanek.fedorapeople.org/elasticsearch/v4/netty-tcnative/netty-tcnative.spec srpm: https://jvanek.fedorapeople.org/elasticsearch/v4/netty-tcnative/netty-tcnative-1.1.30-0.fc21.src.rpm updated patch: https://jvanek.fedorapeople.org/elasticsearch/v4/netty-tcnative/fixLibNames.patch.in The test file is still valid, but only for manual work.I had verified on x86_64 system. Then both my mock roots failed with brokne gcc/libtools[1] https://jvanek.fedorapeople.org/elasticsearch/v4/netty-tcnative/CheckLibrary.java So I dont have RPMS for you this time, and if you wont to rebuild, dont clean your mock-roots:) All issues shuld be fixed. [1] Error: Package: libtool-2.4.2-31.fc22.x86_64 (fedora) Requires: gcc = 4.9.2 Installed: gcc-5.0.0-0.7.fc22.x86_64 (@fedora) gcc = 5.0.0-0.7.fc22 -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1187713] netty-tcnative
https://bugzilla.redhat.com/show_bug.cgi?id=1187713
--- Comment #12 from jiri vanek ---
(In reply to Zbigniew Jędrzejewski-Szmek from comment #11)
> The major question is whether this package should be built at all, i.e. if
> the fork is substantial enough. From what I can see, the answer is yes: the
> sources are completely rearranged, the build system is different, and
> according to http://netty.io/wiki/forked-tomcat-native.html there are
> various fixes.
yes. Thank you for confirmation.
>
> Second question is what to do with the guidelines which say that java
> libraries should not be installed in %{_libdir}. Personally, I think that
> those guidelines are a waste of packagers' time. But since it's the law, I
> would very much prefer to abide by them, unless it is impossible for some
> reason. Looking at o.a.t.j.Library.java in the sources, it should be a
> trivial patch. If you go for the Library() constructor, it should be a
> one-line thing.
Ok I will revisit.
>
> .la files are generally considered harmful. Is there some specific reason to
> install it?
No. It was in build image. I will remove it.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1187713] netty-tcnative
https://bugzilla.redhat.com/show_bug.cgi?id=1187713
--- Comment #11 from Zbigniew Jędrzejewski-Szmek ---
The major question is whether this package should be built at all, i.e. if the
fork is substantial enough. From what I can see, the answer is yes: the sources
are completely rearranged, the build system is different, and according to
http://netty.io/wiki/forked-tomcat-native.html there are various fixes.
Second question is what to do with the guidelines which say that java libraries
should not be installed in %{_libdir}. Personally, I think that those
guidelines are a waste of packagers' time. But since it's the law, I would very
much prefer to abide by them, unless it is impossible for some reason. Looking
at o.a.t.j.Library.java in the sources, it should be a trivial patch. If you go
for the Library() constructor, it should be a one-line thing.
.la files are generally considered harmful. Is there some specific reason to
install it?
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1187713] netty-tcnative
https://bugzilla.redhat.com/show_bug.cgi?id=1187713 --- Comment #10 from jiri vanek --- (In reply to jiri vanek from comment #4) > Updated packages/sources: > > src rpm: > https://jvanek.fedorapeople.org/elasticsearch/v4/netty-tcnative/netty- > tcnative-1.1.30-0.fc22.src.rpm > > spec: > https://jvanek.fedorapeople.org/elasticsearch/v4/netty-tcnative/netty- > tcnative.spec > > rpms: https://jvanek.fedorapeople.org/elasticsearch/v4/netty-tcnative/ > > I hope also the native lib issues are fixed (and of course all your nits). > Please note following two sources were added: > https://jvanek.fedorapeople.org/elasticsearch/v4/netty-tcnative/fixLibNames. > patch > Its pure existence is really really suspiciius! > But see the test... > https://jvanek.fedorapeople.org/elasticsearch/v4/netty-tcnative/CheckLibrary. > java > and its etting in %check... HMM:( https://github.com/netty/netty-tcnative/issues/23 -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1187713] netty-tcnative
https://bugzilla.redhat.com/show_bug.cgi?id=1187713
--- Comment #9 from Mikolaj Izdebski ---
(In reply to jiri vanek from comment #7)
> (In reply to Mikolaj Izdebski from comment #6)
> > Also note that this is a fork of tomcat-native, which is already included in
> > Fedora.
>
> Sure, but the reason for inclusion is here.
Sure, but I would consider adding required changes to tomcat-native (if
possible) instead of packaging a fork.
(In reply to jiri vanek from comment #8)
> * Usage of system.load will require much more patching
It's 10-line patch, not that much IMHO. An example is provided in packaging
guidelines I linked above.
> * tomcat is putting its libraris directly to %{_libdir} too. As this is
> for, I inclined to keep the same schema.
That's a bug in tomcat-native. There is no reason to duplicate the same bug in
another package.
> Those two points are strongly connected, and I have been really thinking
> which one to use. Really a lot. Tried this and yours agian mine and then
> that.. At the end this was most strightforwad
> * little of patching
> * most imiliar to upstream behaviour
> * most close to tomcat.
>
> Unless you insists, I would like to keep with this approach.
The right way is the one described in packaging guidelines - not putting .so
file directly in %{_libdir} and loading it using System.load(). But OFC Zbyszek
has the final word on this.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1187713] netty-tcnative
https://bugzilla.redhat.com/show_bug.cgi?id=1187713
--- Comment #8 from jiri vanek ---
(In reply to Mikolaj Izdebski from comment #5)
> First, Java native libraries (JNI) should not be installed directly in
> %{_libdir} - they are not usable outsides Java world and as such they should
> be placed in subdirectory of %{_libdir}. Secondly, packages in Fedora should
> use System.load() instead of System.loadLibrary() to load JNI libraries. See:
> http://fedoraproject.org/wiki/Packaging:Java#Packaging_JAR_files_that_use_JNI
Yes. But:
* Usage of system.load will require much more patching
* tomcat is putting its libraris directly to %{_libdir} too. As this is for,
I inclined to keep the same schema.
Those two points are strongly connected, and I have been really thinking which
one to use. Really a lot. Tried this and yours agian mine and then that.. At
the end this was most strightforwad
* little of patching
* most imiliar to upstream behaviour
* most close to tomcat.
Unless you insists, I would like to keep with this approach.
>
> It would also be good to run included tests or at least comment why they are
> skipped.
Ugh. I will double check this.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1187713] netty-tcnative
https://bugzilla.redhat.com/show_bug.cgi?id=1187713 --- Comment #7 from jiri vanek --- (In reply to Mikolaj Izdebski from comment #6) > Also note that this is a fork of tomcat-native, which is already included in > Fedora. Sure, but the reason for inclusion is here. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1187713] netty-tcnative
https://bugzilla.redhat.com/show_bug.cgi?id=1187713 --- Comment #6 from Mikolaj Izdebski --- Also note that this is a fork of tomcat-native, which is already included in Fedora. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1187713] netty-tcnative
https://bugzilla.redhat.com/show_bug.cgi?id=1187713
Mikolaj Izdebski changed:
What|Removed |Added
CC||mizde...@redhat.com
--- Comment #5 from Mikolaj Izdebski ---
First, Java native libraries (JNI) should not be installed directly in
%{_libdir} - they are not usable outsides Java world and as such they should be
placed in subdirectory of %{_libdir}. Secondly, packages in Fedora should use
System.load() instead of System.loadLibrary() to load JNI libraries. See:
http://fedoraproject.org/wiki/Packaging:Java#Packaging_JAR_files_that_use_JNI
It would also be good to run included tests or at least comment why they are
skipped.
--
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1187713] netty-tcnative
https://bugzilla.redhat.com/show_bug.cgi?id=1187713 --- Comment #4 from jiri vanek --- Updated packages/sources: src rpm: https://jvanek.fedorapeople.org/elasticsearch/v4/netty-tcnative/netty-tcnative-1.1.30-0.fc22.src.rpm spec: https://jvanek.fedorapeople.org/elasticsearch/v4/netty-tcnative/netty-tcnative.spec rpms: https://jvanek.fedorapeople.org/elasticsearch/v4/netty-tcnative/ I hope also the native lib issues are fixed (and of course all your nits). Please note following two sources were added: https://jvanek.fedorapeople.org/elasticsearch/v4/netty-tcnative/fixLibNames.patch Its pure existence is really really suspiciius! But see the test... https://jvanek.fedorapeople.org/elasticsearch/v4/netty-tcnative/CheckLibrary.java and its etting in %check... HMM:( -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1187713] netty-tcnative
https://bugzilla.redhat.com/show_bug.cgi?id=1187713 --- Comment #3 from Zbigniew Jędrzejewski-Szmek --- (In reply to jiri vanek from comment #2) > Thank you for review. I will get back to this next week. However. You are > writing this should be made noarch - but in fact, it is native package. > But... my jar do not contains (correctly) the native parts. Only classes. > However not even rpm is containing them => So I made it wrongly and forget > to pack the native parts completly! OK. > In next iteration the javadoc and main jar will be noarch, but native lib > will be regular ARCHed one. Maybe if the native part is small (or the jar is small) it doesn't make sense to split. It certainly is not required. -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1187713] netty-tcnative
https://bugzilla.redhat.com/show_bug.cgi?id=1187713 --- Comment #2 from jiri vanek --- Hi! Thank you for review. I will get back to this next week. However. You are writing this should be made noarch - but in fact, it is native package. But... my jar do not contains (correctly) the native parts. Only classes. However not even rpm is containing them => So I made it wrongly and forget to pack the native parts completly! In next iteration the javadoc and main jar will be noarch, but native lib will be regular ARCHed one. As for url, it is : https://github.com/netty/netty-tcnative/archive/netty-tcnative-1.1.30.Fork2.tar.gz not sure how it slipped... mea culpa! -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1187713] netty-tcnative
https://bugzilla.redhat.com/show_bug.cgi?id=1187713
--- Comment #1 from Zbigniew Jędrzejewski-Szmek ---
Remove dot from summary.
BuildRequires: java-devel should not be needed.
===
- All build dependencies are listed in BuildRequires, except for any that are
listed in the exceptions section of Packaging Guidelines.
Note: These BR are not needed: make tar
See: http://fedoraproject.org/wiki/Packaging/Guidelines#Exceptions_2
In principle this could be removed, but is allowed.
[ ]: Package requires other packages for directories it uses.
Note: No known owner of /usr/share/maven-poms/netty-tcnative,
/usr/lib/java/netty-tcnative
[ ]: Package must own all directories that it creates.
Note: Directories without known owners: /usr/share/maven-poms/netty-
tcnative, /usr/lib/java/netty-tcnative
Java:
[x]: Bundled jar/class files should be removed before build
[x]: Packages have proper BuildRequires/Requires on jpackage-utils
Note: Maven packages do not need to (Build)Require jpackage-utils. It is
pulled in by maven-local
Please remove.
[x]: Javadoc documentation files are generated and included in -javadoc
subpackage
Yes.
[x]: Javadoc subpackages should not have Requires: jpackage-utils
This should be fixed.
[x]: Javadocs are placed in %{_javadocdir}/%{name} (no -%{version} symlink)
= SHOULD items =
Generic:
[!]: Sources can be downloaded from URI in Source: tag
Note: Could not download Source0: https://github.com/netty/netty-
tcnative/releases/tag/netty-tcnative-1.1.30.Fork2.tar.gz
This link does not work.
[ ]: If the source package does not include license text(s) as a separate file
from upstream, the packager SHOULD query upstream to include it.
[ ]: Final provides and requires are sane (see attachments).
No, see above.
[ ]: Fully versioned dependency in subpackages if applicable.
Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in netty-
tcnative-javadoc
OK.
Java:
[ ]: Packages are noarch unless they use JNI
Note: netty-tcnative subpackage is not noarch. Please verify manually
Should be noarch.
= EXTRA items =
Generic:
[!]: Spec file according to URL is the same as in SRPM.
Note: Spec file as given by url is not the same as in SRPM (see attached
diff).
See: (this test has no URL)
[ ]: Large data in /usr/share should live in a noarch subpackage if package is
arched.
Note: Arch-ed rpms have a total of 2396160 bytes in /usr/share
Rpmlint
---
Checking: netty-tcnative-1.1.30-0.fc22.x86_64.rpm
netty-tcnative-javadoc-1.1.30-0.fc22.x86_64.rpm
netty-tcnative-1.1.30-0.fc22.src.rpm
netty-tcnative.x86_64: W: summary-ended-with-dot C Netty-tcnative is a fork of
Tomcat Native.
Should be fixed.
netty-tcnative.x86_64: W: name-repeated-in-summary C Netty-tcnative
Should be fixed.
netty-tcnative.x86_64: W: spelling-error %description -l en_US mavenization ->
magnetization, humanization, maximization
netty-tcnative.x86_64: W: incoherent-version-in-changelog 1.1.30.Fork2.0
['1.1.30-0.fc22', '1.1.30-0']
Should be fixed.
netty-tcnative.x86_64: E: no-binary
netty-tcnative.x86_64: W: only-non-binary-in-usr-lib
netty-tcnative.x86_64: W: no-documentation
netty-tcnative.src: W: summary-ended-with-dot C Netty-tcnative is a fork of
Tomcat Native.
netty-tcnative.src: W: name-repeated-in-summary C Netty-tcnative
netty-tcnative.src: W: spelling-error %description -l en_US mavenization ->
magnetization, humanization, maximization
netty-tcnative.src: W: strange-permission netty-tcnative-1.1.30.Fork2.tar.gz
0640L
netty-tcnative.src: W: invalid-url Source0:
https://github.com/netty/netty-tcnative/releases/tag/netty-tcnative-1.1.30.Fork2.tar.gz
HTTP Error 404: Not Found
3 packages and 0 specfiles checked; 1 errors, 11 warnings.
Rpmlint (installed packages)
Cannot parse rpmlint output:
Diff spec file in url and in SRPM
-
--- /var/tmp/1187713-netty-tcnative/srpm/netty-tcnative.spec2015-01-30
13:38:39.140804828 -0500
+++ /var/tmp/1187713-netty-tcnative/srpm-unpacked/netty-tcnative.spec
2015-01-29 10:33:27.0 -0500
@@ -10,8 +10,4 @@
Source0:
https://github.com/netty/netty-tcnative/releases/tag/%{name}-%{namedversion}.tar.gz
-#dont know how to configure requires, just guessing
-Requires: java-headless
-Requires: apr
-Requires: openssl
BuildRequires: maven-local
@@ -25,8 +21,7 @@
BuildRequires: openssl-devel
BuildRequires: java-devel
-BuildRequires: maven-hawtjni-plugin
#parent pom is needed
BuildRequires: netty
-
+BuildRequires: maven-hawtjni-plugin
%description
Requires
netty-tcnative (rpmlib, GLIBC filtered):
java-headless
jpackage-utils
netty-tcnative-javadoc (rpmlib, GLIBC filtered):
jpackage-utils
Should be removed.
Provides
netty-tcnative:
mvn(io.netty:netty-tcnative)
mvn(io.netty:netty-tcnative:pom:)
netty-tcnative
netty-
[Bug 1187713] netty-tcnative
https://bugzilla.redhat.com/show_bug.cgi?id=1187713 Zbigniew Jędrzejewski-Szmek changed: What|Removed |Added Status|NEW |ASSIGNED CC||zbys...@in.waw.pl Assignee|nob...@fedoraproject.org|zbys...@in.waw.pl Flags||fedora-review? -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 1187713] netty-tcnative
https://bugzilla.redhat.com/show_bug.cgi?id=1187713 jiri vanek changed: What|Removed |Added Blocks||1187718 Referenced Bugs: https://bugzilla.redhat.com/show_bug.cgi?id=1187718 [Bug 1187718] please update netty3 to 3.9.3 (or maybe higher) -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
