Bug 735944
Summary: | Review Request: comex-base - base component for comex project | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Armando Basile <hmandevteam> |
Component: | Package Review | Assignee: | Nobody's working on this, feel free to take it <nobody> |
Status: | CLOSED INSUFFICIENT_DATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | hmandevteam, msuchy, mtasaka, package-review |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2015-08-21 09:22:06 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: |
Description
Armando Basile
2011-09-06 08:51:36 UTC
Spec URL: http://www.integrazioneweb.com/repository/specs/fedora/comex-base.spec SRPM URL: http://www.integrazioneweb.com/repository/srpms/fedora/comex-base-0.1.7.0-2.fc15.src.rpm changes: - added ExclusiveArch - added pcsc-lite, pcsc-lite-devel, pcsc-lite-libs dependency - added %{?_smp_mflags} to make command Well, only did a very quick check * License tag - GPLv2 is chosen for license tag in your spec, however I cannot see any files which specifies the version of GPL in the source. Note that just including GPLv2 license text does not actually specify the version of GPL used in the source. Currently License tag should be "GPL+" * BuildRequires - Currently your srpm won't build on koji. Please make it sure that your srpm builds on koji (or with mockbuild) http://koji.fedoraproject.org/koji/taskinfo?taskID=3336036 It seems mono-devel is needed for BR * Explicit dependency - Note that with adding "BR: mono-devel", rpmbuild will find out mono-related runtime dependency (i.e. Requires) automatically and adds the detected dependencies to binary rpms. So writing "Requires: mono-core log4net" explicitly is not needed (after adding BR: mono-devel) * Requires for main pacakge - Usually Requires for main package on subpackages should be (Epoch:)Version-Release specific and also isa specific https://fedoraproject.org/wiki/Packaging/Guidelines#Requiring_Base_Package * pkgconfig dependency - Explicit "Requires: pkgconfig" is no longer needed on Fedora, it is added automatically by rpmbuild https://fedoraproject.org/wiki/Packaging/Guidelines#Pkgconfig_Files * Deprecated usage Please remove lines which are no longer needed - %if 0%{?fedora} < 13 condition is useless. Fedora's version is at least 14 currently - The following is no longer needed on Fedora (EPEL may still need these) * BuildRoot: line * rm -fr %{buildroot} at the top of %install * %clean section * Directory ownership issue - The directory %{_libdir}/mono/gac/ itself is owned by mono-core and this package should not own this directory itself. - The directory %{_datadir}/%{name}/ is not owned by any packages and this package should own it * Requires: pcsc-lite-devel - Can't this be avoided by patching comex-base/comex-base.dll.config? Note that I will be away till Sunday so it may be that replying to you becomes late. By the way I will appreciate it if you would review my review request (bug 678674) SPEC: http://www.integrazioneweb.com/repository/specs/fedora/comex-base.spec SRPMS: http://www.integrazioneweb.com/repository/srpms/fedora/comex-base-0.1.7.1-1.fc15.src.rpm changes in sources http://code.google.com/p/comex-project/source/detail?r=73: - removed dependence from pcsc-lite devel package - changed alias in .config file (now also .Net see library) - changed DllImport alias for Interop (now also .Net see library) - added license information changes in spec: - in BuildRequires: changed mono-core to mono-devel - in Requires: removed pcsc-lite-devel - in Requires: removed mono-core - in Requires: removed log4net - in Requires: removed pkgconfig - build section: removed "if fedora < 13" condition - removed BuildRoot - install section: removed "rm -fr %{buildroot}" - clean section: removed - files section: changed ref from mono/gac/ to mono/gac/comex-base/ - files section: changed ref from %{name}/Languages/ to %{name}/ about your review request bug 678674, i don't know ruby so i don't know how could i help you. Well, I have not checked your latest srpm, however just for this: (In reply to comment #3) > about your review request bug 678674, i don't know ruby so i don't know how > could i help you. Note that I don't know mono well, either :) > Note that I don't know mono well, either :)
Wow, seems you are familiar with mono
Could try to review a mono package for first ?
I am triaging old review tickets. I can't promise a review if you reply, but by closing out the stale tickets we can devote extra attention to the ones which aren't stale. spec and srpm links are invalid. i updating specs and srmps, thanks for reply SPEC: http://www.integrazioneweb.com/repository/specs/fedora/comex-base.spec SRPMS: http://www.integrazioneweb.com/repository/srpms/fedora/comex-base-0.1.8.5-1.fc18.src.rpm changes in spec: - in build section: set --libdir=%{_prefix}/lib (http://fedoraproject.org/wiki/Packaging:Mono#File_Locations) Some initial comments before I give it a try on Koji: Probably no debuginfo will be created, due to the nature of Mono software: https://fedoraproject.org/wiki/Packaging:Debuginfo?rd=Packaging/Debuginfo#Useless_or_incomplete_debuginfo_packages_due_to_other_reasons This is addressed by your comment in spec line 13. OK so far, but either you uncomment the line (if debuginfo will be usable in this certain case) or you have to escape the % signs with another one to make rpmlint happy. In some odd cases, even macros in comments could be expanded and lead to unexpected results. Requires: %{name} = %{version} has to be Requires: %{name}%{?_isa} = %{version}-%{release} The isa tag needs to be added because your package is multiarch. The release tag makes sure that the -devel package will be updated with the base package. %defattr(-,root,root,-) is applicable for EPEL 5 only. As the second changelog entry says, you don't plan to package for EPEL 5, please remove that lines in the file lists. In the file lists, %{_prefix}/lib has to replaced with %{_libdir}. Otherwise we would get a "hardcoded library path" warning from rpmlint. Or can you explain why you have to use /usr/lib explicitely instead of /usr/lib and /usr/lib64 depending on the package architecture? %changelog * Thu May 02 2013 Armando Basile <hmandevteam> 0.1.8.5-1.fc18 In the %changelog section, it is unneeded to mention the distribution release. A simple "0.1.8.5-1" is sufficient here. Doesn't matter which Fedora release is pointed by your spec, one could rebuild your package for f17 or f19, whatever, so that your "f18" tag becomes useless. Requires: pcsc-lite Requires: pcsc-lite-libs The latter is not needed, it is required by pcsc-lite anyway. You are using both tabs and whitespaces in your spec. It's rather cosmetic, but has to be fixed. I recommend spaces, that way the spec looks the same in any text editor, regardless of the configured tab width. Any news here...? Hi Mario, i will modify specs and resubmit, thanks No response for years. Closing. Feel free to reopen if you want to continue. |