Bug 226223

For 2.14.7-6:

* License
  - Please recheck the license
    * Most source codes do not specify any version and
      it seems that source codes which contain license term contain
      "and any later" term. So at least this can be
      "LGPLv2+ and GPLv2+"

* Redundant requires
  - For devel
--------------------------------------------------
Requires: glib2-devel
Requires: glib2-devel >= %{glib2_version}
--------------------------------------------------
    The first line can be omitted.

* Parallel make
  - Support parallel make when possible.

* %makeinstall
  - Please avoid %makeinstall when possible.
  - Usually I recommend
--------------------------------------------------
make install DESTDIR=$RPM_BUILD_ROOT INSTALL="%{__install} -p"
--------------------------------------------------
    for recent Makefile. This will keep timestamps on
    most files

* Directory ownership issue
  - Please fix up directory ownership issue.
--------------------------------------------------
    %_libdir/orbit-2.0 - must owned by main, not by devel
    %_datadir/idl - must owned by ORBit2-devel, not by
                    libbonobo-devel

* Defattr
  - Now we recommend %defattr(-,root,root,-)

* Rpmlint
  - file-not-utf8 /usr/share/doc/ORBit2-2.14.7/NEWS
    * Please change the encoding to UTF-8

Matthias, ping?

Should all be taken care of in the current build, I think

For 2.14.16-2:

* Licensing
  https://fedoraproject.org/wiki/Packaging/LicensingGuidelines#Multiple_Licensing_Scenarios
  - Please write in the spec file what files are actually
    licensed under GPLv2+.
    As far as I checked the source codes, the codes
    under GPLv2+ are:
------------------------------------------------------
src/idl-compiler/orbit-idl-driver.c
src/idl-compiler/orbit-idl-main.c
src/services/name/name-client.c
test/*.c
test/everything/*.c
test/poa/*.c
------------------------------------------------------
    and these affects files under %_bindir in ORBit2-devel.

    So the license of ORBit2 (main package) should be only
    "LGPLv2+" and only -devel package should have "LGPLv2+
    and GPLv2+" license tag.

! Timestamps
  make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"
  is preferrred to keep timestamps on installed files.

* Requires
  - Would you explain why -devel subpackage should have
    "Requires: indent"?

* Documents
  - Please add the following file(s) into main package
------------------------------------------------------
COPYING.LIB
------------------------------------------------------
   - Maybe the following file(s) can be added in -devel subpackage
------------------------------------------------------
ChangeLog
MAINTAINERS
HACKING
------------------------------------------------------

* Shipping static archive
  https://fedoraproject.org/wiki/Packaging/Guidelines#Packaging_Static_Libraries
  - Split static archives into -static subpackge so that
    we can keep track of packages really needing the archive.

mtasaka,
  Can you please review current rawhide package again? I will do the necessary changes in git.

I too see -static should be generated.

Well, checking master head (2.14.18-2.fc15)

* License
  - So, if you want to use "LGPLv2+ and GPLv2+" for license tag, please
    specify explicitly which part is LGPLv2+ and which part is GPLv2+.

* Version specific BR
  - ">=" parts on BuildRequires's are no longer needed because packages
    on currently supported Fedora branches all satisfy these dependencies
    ( And especially I see no reason to keep writing these version specific BR
      on current rawhide )
    https://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires

* Status of the patches
  - Please write the status of the patches (i.e. if the patch is to be merged
    into the upstream, or if it is Fedora specific, or something else).
    ! Especially, I guess maintainers of ORBit2 on Fedora also take part in
      GNOME Project, so I cannot guess why 2.14.3- patches are not yet merged
      into 2.14.18 tarball.
    https://fedoraproject.org/wiki/Packaging/Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment

* Requires on -devel subpackage
  - Would you explain what "R: indent" is for?
  - "R: pkgconfig" "R: foo-devel" are no longer needed. These dependencies
    are automatically needed.
    ! And as I wrote above, I think keeping writing explicit version dependencies here
      makes no sense.

* Duplicate %description
  - %description in -devel subpackage contains the same contents shown in the %description
    of the main package and it is redundant.

* Timestamps
  - Please consider to add "INSTALL='install -p'" option to "make install" to keep
    timestamps on installed files as much as possible, especially on installed header
    files.

* Static archive
  - Please take care of
    https://fedoraproject.org/wiki/Packaging/Guidelines#Packaging_Static_Libraries
    - At least .a files must be split out from -devel subpackage

* %check
  - As this package contains test/ directory, please add %check section and execute
    some test program (like $ make check ) there
    ! And (on my local machine) actually currently "make check" fails like
-------------------------------------------------------------------------------
mem usage prev/post:  1388KB /  1464KB --  1000x test_ORBit_alloc ()
**
ERROR:test-mem.c:239:main_func: assertion failed: (mem_usage_end - mem_usage_start < 50)
/bin/sh: line 5: 30139 Aborted                 ${dir}$tst
FAIL: test-mem
....
....
1 of 6 tests failed
Please report to http://bugzilla.gnome.org/enter_bug.cgi?product=ORBit2
-------------------------------------------------------------------------------

Just to update here, I am working on this review, meanwhile some modification are available at

SRPM: http://paragn.fedorapeople.org/ORBit2-2.14.18-3.fc15.src.rpm
SPEC: http://paragn.fedorapeople.org/ORBit2.spec

Unfortunately, I have no time to follow this package. Only thing I guess remaining here is to make sure patches have upstream bugzilla reported and here is only Patch1 need to have bug id and also %check thing.

Removing from this package review work.

Fresh review:

- rpmlint checks return:

ORBit2.spec:389: W: macro-in-%changelog %{_datadir}
Macros are expanded in %changelog too, which can in unfortunate cases lead to
the package not building at all, or other subtle unexpected conditions that
affect the build.  Even when that doesn't happen, the expansion results in
possibly "rewriting history" on subsequent package revisions and generally odd
entries eg. in source rpms, which is rarely wanted.  Avoid use of macros in
%changelog altogether, or use two '%'s to escape them, like '%%foo'.

ORBit2.spec:551: W: macro-in-%changelog %files
Macros are expanded in %changelog too, which can in unfortunate cases lead to
the package not building at all, or other subtle unexpected conditions that
affect the build.  Even when that doesn't happen, the expansion results in
possibly "rewriting history" on subsequent package revisions and generally odd
entries eg. in source rpms, which is rarely wanted.  Avoid use of macros in
%changelog altogether, or use two '%'s to escape them, like '%%foo'.

ORBit2.spec:551: W: macro-in-%changelog %{prefix}
Macros are expanded in %changelog too, which can in unfortunate cases lead to
the package not building at all, or other subtle unexpected conditions that
affect the build.  Even when that doesn't happen, the expansion results in
possibly "rewriting history" on subsequent package revisions and generally odd
entries eg. in source rpms, which is rarely wanted.  Avoid use of macros in
%changelog altogether, or use two '%'s to escape them, like '%%foo'.

Trivial to fix.

Some no man page and wrong fsf address errors, fix if feasible.

- package meets naming guidelines
- package meets packaging guidelines
- license ( LGPLv2+ and GPLv2+ ) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream
- package compiles on devel (x86_64)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file
- devel package ok
- no .la files
- post/postun ldconfig ok
- devel requires base package n-v-r 

So trivial things, and Parag's comments on patches.

Committed macro fixes, patch bits have been done, make check is still broken, can you look into it?

And now it doesn't build.  All I did was escape commented macros.

http://koji.fedoraproject.org/koji/getfile?taskID=4024931&name=build.log

I'll have a look.

And now, with no changes, it's fine.

Make check is optional, still broken, closing.  APPROVED.