Bug 225771
Summary: | Merge Review: fribidi | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Nobody's working on this, feel free to take it <nobody> |
Component: | Package Review | Assignee: | Caolan McNamara <caolanm> |
Status: | CLOSED RAWHIDE | QA Contact: | Fedora Package Reviews List <fedora-package-review> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | caolanm, michel, roozbeh |
Target Milestone: | --- | Flags: | roozbeh:
fedora-review-
kevin: fedora-cvs+ |
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2007-05-16 12:40:46 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
Nobody's working on this, feel free to take it
2007-01-31 18:40:15 UTC
Taking this for review, as I maintain the pyfribidi package that depends on this. GOOD ==== MUST: rpmlint output fine: $ rpmlint fribidi-devel-0.10.7-5.1.i386.rpm W: fribidi-devel no-documentation MUST: spec filename matches %{name} MUST: package is free software MUST: License matches actual license (LGPL) MUST: text of license in both upstream and package as %doc MUST: spec file legible MUST: no ExcludeArch MUST: BuildRequires assumed fine MUST: no localization MUST: ldconfig used fine MUST: not relocatable MUST: owned dirs fine (-devel creates /usr/lib and depends on pkgconfig which owns it) MUST: no dup files MUST: clean section fine MUST: macro use consistent MUST: contains code MUST: no large documentation MUST: %docs don't affect runtime MUST: header files and static libs in -devel MUST: -devel which has *.pc Req's pkgconfig\ MUST: *.la explicitly removed MUST: not GUI MUST: does not own other's dirs SHOULD: no scriptlets SHOULD: no subpackages other than -devel SHOULD: *.pc files in -devel BAD === MUST: Package Naming Guidelines * release should use integers and dist-tags. should be changed to 6%{?dist} MUST: Packaging Guidelines * BuildRoot should be: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) * "make" should be changed to "make %{?_smp_mflags}" * "%makeinstall" should be changed to "make DESTDIR=$RPM_BUILD_ROOT install" MUST: US English * FriBidi should be spelled with capital F and B in summary and description field of -devel subpackage. * Hebrew and Arabic should be spelled with capital H and A in descriptionp of main package. * "eg." should be changed to "e.g." or better, "for example": "for example Arabic and Hebrew"). * Static library (*.a) should not be packaged unless there is a very good reason for packaging it. MUST: source to match upstream * source matches upstream (md5sum checked) but Source: should be change to include the full URL: http://fribidi.org/download/fribidi-[...] MUST: file permissions * Please use %defattr(-,root,root,-) instead of %defattr(-,root,root) (it's used twice). MUST: devel packages must require fully versioned dependency * -devel should do this instead of just %{name} = %{version}: Requires: %{name} = %{version}-%{release} a) actually I think people are gone a little mad on using the dist tag: http://fedoraproject.org/wiki/Packaging/NamingGuidelines says "If you wish to use a single spec file to build for multiple distributions, you can use the %{?dist} tag in the Release field", implies that it's not a requirement, but allowed if you want to use it. Anyway, doesn't bother me either way, so added along with the rest of the review points (I think I got them all) as fribidi-0_10_7-6_fc7 ( Just a note: * As written in http://fedoraproject.org/wiki/Packaging/DistTag , using ?dist tag is "preferred", _not_ mandatory. There are some cases in that ?dist tag is of no means. * As written in http://fedoraproject.org/wiki/Packaging/Guidelines , setting buildroot as %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) is "preferred", and also this is _not_ mandatory. Making to use %__id_u mandatory is still under discussion, some reviewers and sponsors strongly disagree this. ) (In reply to comment #4) > * As written in http://fedoraproject.org/wiki/Packaging/Guidelines , > setting buildroot as > %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) > is "preferred", and also this is _not_ mandatory. > Making to use %__id_u mandatory is still under discussion, > some reviewers and sponsors strongly disagree this. This has changed: last week, the FPC has decided to make it mandatory. (In reply to comment #5) > (In reply to comment #4) > > > * As written in http://fedoraproject.org/wiki/Packaging/Guidelines , > > setting buildroot as > > %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) > > is "preferred", and also this is _not_ mandatory. > > Making to use %__id_u mandatory is still under discussion, > > some reviewers and sponsors strongly disagree this. > This has changed: last week, the FPC has decided to make it mandatory. I think this bug is not a preferred place to discuss, however, while I don't disagree (rather I recommend) to use __id_u, what I fear that the macro for _id_u is as written in /usr/lib/rpm/macros, ------------------------------------------------ # # fixowner, fixgroup, and fixperms are run at the end of hardcoded setup # These macros are necessary only for legacy compatibility, and have moved # to per-platform macro configuration (i.e. /usr/lib/rpm/<arch>-<os>/macros) # # Note: These are no longer enabled by default. #%__id_u %{__id} -u #%__chown_Rhf %{__chown} -Rhf #%__chgrp_Rhf %{__chgrp} -Rhf #%_fixowner [ `%{__id_u}` = '0' ] && %{__chown_Rhf} root #%_fixgroup [ `%{__id_u}` = '0' ] && %{__chgrp_Rhf} root #%_fixperms %{__chmod} -Rf a+rX,u+w,g-w,o-w # -------------------------------------------------- ... it seems that this is written for _legacy_ support, and these macros may be removed in the future. Note: on rpm 4.4.8, _id_u macro is for redhat _only_. Hello, Is this review officially completed? Also, are the maintainers interested in branching it for EL-5? I need it to update EL-5's fbreader. Thanks! Package Change Request ====================== Package Name: fribidi New Branches: EL-5 EL-6 Owners: salimma See comment above -- needed for newer fbreader releases CVS done (by process-cvs-requests.py). |